How to avoid UNLINK security risks in PHP?

StackOverflow https://stackoverflow.com/questions/3415418

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();
    }
Was it helpful?

Solution

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.

OTHER TIPS

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

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.

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.

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.

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

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)

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top