6

I'm using UNLINK with PHP and AJAX. I know that in this way is very dangerous, because everyone can delete any files. But I need to use AJAX because I can't reload the page when I delete the files.

So how should I do to allow to delete the file only for the user who owns it?

Please let me know other things too if you think I'm doing here something wrong or something else what you have in mind and you think that it will be useful : )

My PHP code:


<?php

    $photo_id       = $_GET['photo_id'];
    $thumbnail_id   = $_GET['thumbnail_id'];    

    function deletePhotos($id){
        return unlink($id);
    }

    if(isset($photo_id)){
        deletePhotos($photo_id);
    }
    if(isset($thumbnail_id)){
        deletePhotos($thumbnail_id);
    }


 ?>

My AJAX code:


function deletePhoto(photo, thumbnail){

        var photos = encodeURIComponent(photo);
        var thumbnails = encodeURIComponent(thumbnail);

        if (window.XMLHttpRequest) {// code for IE7+, Firefox, Chrome, Opera, Safari
          xmlhttp=new XMLHttpRequest();
        } else {// code for IE6, IE5
          xmlhttp=new ActiveXObject("Microsoft.XMLHTTP");
        }

        xmlhttp.onreadystatechange=function() {
            if (xmlhttp.readyState==4 && xmlhttp.status==200) {
                document.getElementById("media").innerHTML=xmlhttp.responseText;
            }
        }
        xmlhttp.open("GET", "http://192.168.2.104/images/users/delete_photo.php?photo_id="+photos+"&thumbnail_id="+thumbnails, true);
        xmlhttp.send();
    }
6
  • 1
    AJAX has nothing to do with security. From the server point of view, AJAX call is no different from a regular one. Your problem not in AJAX but in lack of authorization. Sooner you understand it, sooner you solve your problem. Aug 5, 2010 at 14:23
  • Hi @Col. Shrapnel, I don't think you are totally right, because without AJAX I don't need to make a file what anyone can access and can delete anything with a GET request. Otherwise I know that the problem here is with authorization that's why I've asked this question So how should I do to allow to delete the file only for the user who owns it? Aug 5, 2010 at 14:31
  • 1
    how can you let user delete a file without such a script? Aug 5, 2010 at 14:46
  • Not without such a script but without GET. Aug 5, 2010 at 16:23
  • So what? think POST request shouldn't be secured? lol Aug 5, 2010 at 17:53

7 Answers 7

9

You need to authenticate the user somehow.

Your user needs to be authenticated with a username and a password.

PHP session can be used to remember, and you should use a database table or a text file on the server to store file ownership information.

Then, before unlinking anything, your logic should make sure that the currently "authenticated" user is the owner of the file.

7
  • If you mean logging in, then the user is logged in. If I don't allow users to access the file who's not logged in that's a bit better, but users who are logged in they still can delete each others files. Aug 5, 2010 at 14:05
  • that's why you need to store file ownership in another table, and before unlinking anything you make sure that the "authenticated" user is the owner of the file.
    – Wadih M.
    Aug 5, 2010 at 14:07
  • the only sensible answer here. @CIRK listen to that comment above. that's the only solution. You're going totally wrong way. AJAX is not your problem Aug 5, 2010 at 14:24
  • Yes this is a logical answer, the problem with my overall code that in this stage I don't send any data to the database, but I think I can figure out so I will try it. Aug 5, 2010 at 14:34
  • 1
    @CIRK yes, a perfect word - logical. without storing information if the file owner, you cannot verify a file owner. Very logical, isn't it? Aug 5, 2010 at 14:39
4

you can simplify your task by using a very simple database substitution - a directory structure. keep user's files in user's directory. so, you can always check if particular user has rights to delete. Name a directory after user's name, or - much better - numeric user id

just something like

$photo_id = basename($_GET['photo_id'];)
$filename = $filebase.$_SESSION['user_id']."/".$photo_id;
if (file_exists($filename) unlink ($filename);
2

Limit the unlinking to the directory with the photos. That is, do not allow .. in the path, or check the full path after doing realpath(). Otherwise, the user can request delete_photo.php?photo_id=../../../../etc/passwd and break the system.

4
  • If they encode '..' in a different character set, it could potentially pass through. I've read about it in the past.
    – Wadih M.
    Aug 5, 2010 at 14:06
  • @Wadih yes, but that was a flaw (I think in Apache) that was fixed since.
    – Pekka
    Aug 5, 2010 at 14:07
  • 1
    If php is running as root i think you have bigger problems on your hands.
    – rook
    Aug 5, 2010 at 15:13
  • Indeed, because PHP is not root the example of /etc/passwd is exaggerated and does not really work.
    – Sjoerd
    Aug 6, 2010 at 7:03
1

In your PHP:

  • Make sure $_GET['photo_id'] and $_GET['thumbnail_id'] don't contain "../"
  • Also make sure you prepend a basepath to the ID.

Otherwise users can delete any file.

As for the ownership, you have to store the information who owns which file somewhere on the server side (for example a MySql-DB). Then you should consult this location before deleting the file.

0

As Wadih M. has said. You need to authenticate your user. Then you can use that to compare the "Owner of the Image" to the "User currently log in". This will give you all the security you may want.

As I said before, name the varaibles so that they sound right. When I see "id" in a varaiable. I automatically assume as a programmer that it is a numeric var.

5
  • The ID is not a number :D it's the file_name with some unique stuff before them something like efb03_orange.png. The problem is that in this section I haven't sent anything yet to the database. So I don't know how to check if the logged in user is the user who owns the file. Aug 5, 2010 at 14:22
  • I think that kind of breaking naming convention for the var. Should be like $imagePlaceholder. I will do further updates in answer to your comment.
    – Anraiki
    Aug 5, 2010 at 14:38
  • I just notice that... making the person jump over more hoop may not be necessary once the "verification" of the owner of the file goes through.
    – Anraiki
    Aug 5, 2010 at 15:03
  • 1
    THE ABOVE IS A VERY DANGEROUS PIECE OF CODE. because: people can put a %00 in $_GET['photo'], and since php doesn't care about the 0 byte but all underlying libc functions do you can unlink any existing file. Try this: <?php var_dump(file_exists("/etc/passwd\0foo.gif")); ?> so I repeat: just put it in a database unless you are really sure you ruled out all possible loopholes, which you almost cannot do
    – mvds
    Aug 5, 2010 at 19:18
  • When you said "you", are you particularly referring to me or everyone? The code is dangerous if taken lightly.
    – Anraiki
    Aug 6, 2010 at 14:48
0

have had the same problem and got around it using PHP's ftp_delete function

-1

A different suggestion: don't store files on disk, but put them in a database. This keeps a very clear distinction between your site+scripts and "user data".

(someone once told me that files were files, and databases were for data, and those are different, but as I see it, files contain data anyway. mysql has a perfect LONGBLOB type to put anything in, and you can store meta-data, such as file-type and filename, in separate fields in the same data row, which keeps things clean and simple)

5
  • In the case of images I don't think it's a good idea to store them in the database (for performance reasons). Cause when displaying them you need a PHP script to read from the database. As Images are seperate HTTP requests this will result in multiple connections to the database, that need to be established.
    – JochenJung
    Aug 5, 2010 at 14:16
  • Fyi, Microsoft SharePoint 3.0 does that (stores files in the DB). I don't necessarily agree with this decision, as I'm a believer that 'file system' should be used to store 'files', and 'databases' for tabular data. But this design pattern would make sense in some scenarios.
    – Wadih M.
    Aug 5, 2010 at 15:15
  • Please explain to me how a filesystem is not a database as well? At non-exotic volumes, performance is certainly not a problem, judging from experience. The database connection can persist between requests. You have to weigh the downside (performance) to the upsides (ease of maintenance, backup, data consistency, security aspects as in the original question). Think about a mysql replication setup, the horror involved to also replicate the files!
    – mvds
    Aug 5, 2010 at 15:25
  • @mvds I agree, maintaining file binary data and file meta data through the file system can be non-trivial especially when cross-platform communication is happening (linux, windows, etc). Both technologies have their own advantages/disadvantages.
    – Wadih M.
    Aug 5, 2010 at 18:57
  • @Wadih: fortunately not all of us have facebook or youtube-like proportions, so the only upside of filesystem-files (performance) is not needed. Leaves a lot of tricky points - see my comment to @anraiki about 0 bytes in strings, in php vs the underlying libc functions.
    – mvds
    Aug 5, 2010 at 19:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.