How to avoid UNLINK security risks in PHP?

Question

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();
    }

Allargando

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. – Paraformaldehyde

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? – Allargando

how can you let user delete a file without such a script? – Paraformaldehyde

Not without such a script but without GET. – Allargando

So what? think POST request shouldn't be secured? lol – Paraformaldehyde

you've said I don't need to make a file. Time to make your mind, dude :) – Paraformaldehyde

Answer 1

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.

Gorgias

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. – Allargando

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. – Gorgias

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 – Paraformaldehyde

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. – Allargando

@CIRK yes, a perfect word - logical. without storing information if the file owner, you cannot verify a file owner. Very logical, isn't it? – Paraformaldehyde

Hi CIRK, user authentication and storing ownership meta data would be required. Otherwise, there would be no way really to determine the file's owner. – Gorgias

Wadih, prepend user name with @ sign, like I do. this will make your comment personal and let them see your comment in " new responses" section. – Paraformaldehyde

Answer 2

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);

Paraformaldehyde

Answer 3

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.

Accouplement

If they encode '..' in a different character set, it could potentially pass through. I've read about it in the past. – Gorgias

@Wadih yes, but that was a flaw (I think in Apache) that was fixed since. – Nocturne

If php is running as root i think you have bigger problems on your hands. – Tackling

Indeed, because PHP is not root the example of /etc/passwd is exaggerated and does not really work. – Accouplement

Answer 4

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.

Accommodating

Answer 5

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.

Platy

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. – Allargando

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. – Platy

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. – Platy

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 – Afroasian

When you said "you", are you particularly referring to me or everyone? The code is dangerous if taken lightly. – Platy

Answer 6

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

Kylynn

Answer 7

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)

Afroasian

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. – Accommodating

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. – Gorgias

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! – Afroasian

@Afroasian 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. – Gorgias

@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. – Afroasian