Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Results 1 to 11 of 11
  1. #1
    Senior Coder doubledee's Avatar
    Join Date
    Mar 2011
    Location
    Arizona
    Posts
    1,071
    Thanks
    26
    Thanked 0 Times in 0 Posts

    Safely Deleting a File?

    My website allows members to upload a photo of themselves. Just one.

    I spent an *enormous* amount of time writing a bullet-proof script to make sure that no one uploads something they shouldn't.

    But now I have one problem that I want to be very careful about...

    Currently, if a member uploads a replacement photo, my script uploads the new photo, but leaves the old one in the "uploads" directory.

    I want to add on to my script to either 1.) Delete the old photo, or 2.) Move the old photo to a "delete" directory so that I know what needs to be cleaned out.

    Can someone give some ideas on how to do this, and prevent hackers having the ability to tamper with or destroy other files on my website or web server?!

    Sincerely,


    Debbie

  • #2
    Regular Coder
    Join Date
    Sep 2011
    Posts
    408
    Thanks
    18
    Thanked 26 Times in 26 Posts
    Save the photo as their username. If they upload a new one then it will just overwrite the old one. Otherwise, you'll have to make a table in a database to match photos to users and that would just take up unnecessary space.
    If I've helped you out, show your appreciation by clicking the "Thanks" link as well as a link below!

    AdFly
    Facebook | Twitter
    Google | YouTube

  • #3
    Senior Coder doubledee's Avatar
    Join Date
    Mar 2011
    Location
    Arizona
    Posts
    1,071
    Thanks
    26
    Thanked 0 Times in 0 Posts
    Quote Originally Posted by Dubz View Post
    Save the photo as their username. If they upload a new one then it will just overwrite the old one. Otherwise, you'll have to make a table in a database to match photos to users and that would just take up unnecessary space.
    Dubz, I luv your responses, but you fell short this time.

    (Maybe my OP wasn't worded in enough detail?)

    I believe PHP provides the ability to delete files off of the file system and also to move files around the file system.

    That is giving A LOT of power to a hacker!!!

    I am trying to get advice on what is the *safest* way to use PHP to delete the old uploaded photo from my file system, while preventing a hacker from changing something and deleting whatever they want.

    (My upload script is over 1,000 lines of code, and providing safe uploads in PHP is a VERY IN-DEPTH topic for those who really understand things.)

    The only thing riskier than letting strangers upload files onto my web server, is (indirectly) giving them the ability to MOVE or DELETE files!!

    And so I don't want to defeat my 1,000 lines of upload awesomeness but writing lame code to get rid of the old photo!!

    Does that make more sense?

    Sincerely,


    Debbie

  • #4
    Regular Coder
    Join Date
    Sep 2011
    Posts
    408
    Thanks
    18
    Thanked 26 Times in 26 Posts
    They aren't the ones moving or deleting the files, your code is. If you use unlink() you can delete files, as for moving you may use move_uploaded_file().

    If you save the image by the username, they can't change it so they can't mess with that. Uploading a new image and saving it as their username would overwrite the old one, unless they changed the extension, which is why you should delete the old one as well, or scan for any files with the same name and delete them.

    If that's still too scary for you, perhaps you can load the images outside the web access folder (ex. www, public_html, or html) and then use a php script to get the image and send it to them. Make the images/directory read only so whatever "images" they upload won't be executed in any way.

    I'm sure there's some tool out there to check the integrity of the image though and make sure it is an image.

    If that's not what you're asking for then I don't know what else to say, maybe someone else will have better luck
    If I've helped you out, show your appreciation by clicking the "Thanks" link as well as a link below!

    AdFly
    Facebook | Twitter
    Google | YouTube

  • #5
    Senior Coder
    Join Date
    Feb 2011
    Location
    Your Monitor
    Posts
    4,313
    Thanks
    58
    Thanked 525 Times in 512 Posts
    Blog Entries
    5
    Quote Originally Posted by doubledee View Post
    Dubz, I luv your responses, but you fell short this time.

    (Maybe my OP wasn't worded in enough detail?)
    LOL!

    Quote Originally Posted by doubledee View Post
    I believe PHP provides the ability to delete files off of the file system and also to move files around the file system.
    Yes it does. move_uploaded_file() for files sent via post submission (which are then kept in the $_FILES array), copy() copies file from one location to another, unlink() deletes, rename() speaks for itself.

    Quote Originally Posted by doubledee View Post
    That is giving A LOT of power to a hacker!!!
    Only if they can find a weakness in your code to exploit. Typically this is via the database or a form / email injection and not normally direct access to the file system. That said it's not impossible, I did have a site hacked a year or two back that stood up to the hackers for over a year before they brought it down to it's knees. It never had any user forms etc - minimal user input and yet they hit the filesystem (unfortunately the site owners never told me this site was being repeatedly targetted before hiring me otherwise I'd have turned the job away but it still stumped the attackers for a long time!). Typically however a server can be breached using other means and not just hacking the site by forms / injection. FTP is a weak point if the server allows repeated login attempts, if there is a shell account on the box that is another potential weakpoint... IF they can get into the main server your form and php security means nothing as they have full access to everything.

    Quote Originally Posted by doubledee View Post
    (My upload script is over 1,000 lines of code, and providing safe uploads in PHP is a VERY IN-DEPTH topic for those who really understand things.)
    What on earth are you doing in there? lol. You also need to remember that more code = more CPU usage = slower performance with more users.

    Quote Originally Posted by Dubz View Post
    They aren't the ones moving or deleting the files, your code is. If you use unlink() you can delete files, as for moving you may use move_uploaded_file().
    move_uploaded_file() is only going to do that - move an uploaded file. It will not move a currently existing (old) file to another location on the server. copy() and then unlink() is best for that.


    Quote Originally Posted by Dubz View Post
    If that's not what you're asking for then I don't know what else to say, maybe someone else will have better luck
    lol you're learning
    Last edited by tangoforce; 06-15-2014 at 09:45 PM.
    See my new CodingForums Blog: http://www.codingforums.com/blogs/tangoforce/

    Many useful explanations and tips including: Cannot modify headers - already sent, The IE if (isset($_POST['submit'])) bug explained, unexpected T_CONSTANT_ENCAPSED_STRING, debugging tips and much more!

  • #6
    Senior Coder doubledee's Avatar
    Join Date
    Mar 2011
    Location
    Arizona
    Posts
    1,071
    Thanks
    26
    Thanked 0 Times in 0 Posts
    Quote Originally Posted by Dubz View Post
    They aren't the ones moving or deleting the files, your code is.
    Right, but I want to be doubly careful that I don't give a hacker some way to start deleting other peoples photos or system files.


    Quote Originally Posted by Dubz View Post
    If you use unlink() you can delete files, as for moving you may use move_uploaded_file().
    I am aware of those functions, and that is the area where I want some advice.


    BTW, I have literally spent all Sunday re-reading my upload.php code and getting back into my code after nearly 6 months of sitting on the shelf. Wow, what a read!!!

    Now that I have all of my code "loaded into (Debbie's) memory", I am ready to figure out how to *safely* delete old photos.


    Quote Originally Posted by Dubz View Post
    If you save the image by the username, they can't change it so they can't mess with that. Uploading a new image and saving it as their username would overwrite the old one, unless they changed the extension, which is why you should delete the old one as well, or scan for any files with the same name and delete them.
    You are totally losing me here?!

    What does the name of a user's photo have to do with security???

    The approach I take is...
    PHP Code:
        // Hash Uploaded File.    
        
    $photoHash hash_file('sha256'$tempFileFALSE); 


    Quote Originally Posted by Dubz View Post
    If that's still too scary for you, perhaps you can load the images outside the web access folder (ex. www, public_html, or html) and then use a php script to get the image and send it to them. Make the images/directory read only so whatever "images" they upload won't be executed in any way.
    Well, that is another topic I wonder about...

    Is it safe enough to allow user uploaded photos in the Web Root, or should they go outside of the Web Root?


    Quote Originally Posted by Dubz View Post
    I'm sure there's some tool out there to check the integrity of the image though and make sure it is an image.

    If that's not what you're asking for then I don't know what else to say, maybe someone else will have better luck
    The two functions you mention above are exactly in the area where I want some advice.

    I just recall reading that using unlink can be dangerous if not implemented properly.

    Now that it is past 10:00pm, and I'm exhausted, I probably won't get much more done tonight.

    But tomorrow I will see what the safest way is to incorporate one of those two functions, and see what you guys think.

    Sincerely,


    Debbie

  • #7
    Senior Coder doubledee's Avatar
    Join Date
    Mar 2011
    Location
    Arizona
    Posts
    1,071
    Thanks
    26
    Thanked 0 Times in 0 Posts
    Quote Originally Posted by tangoforce View Post
    Only if they can find a weakness in your code to exploit. Typically this is via the database or a form / email injection and not normally direct access to the file system. That said it's not impossible, I did have a site hacked a year or two back that stood up to the hackers for over a year before they brought it down to it's knees. It never had any user forms etc - minimal user input and yet they hit the filesystem (unfortunately the site owners never told me this site was being repeatedly targetted before hiring me otherwise I'd have turned the job away but it still stumped the attackers for a long time!). Typically however a server can be breached using other means and not just hacking the site by forms / injection. FTP is a weak point if the server allows repeated login attempts, if there is a shell account on the box that is another potential weakpoint... IF they can get into the main server your form and php security means nothing as they have full access to everything.
    Security worries me soo much. (Seems like there is no way to stop the bad guys these days?!)


    Quote Originally Posted by tangoforce View Post
    What on earth are you doing in there? lol. You also need to remember that more code = more CPU usage = slower performance with more users.
    My upload script is kick butt, and won't make the CPU even break a sweat.

    Guess this makes me a true geek, but I thoroughly enjoyed re-reading my code today!

    It took me all day and evening, but I *get* what I was doing before, and everything looks good as-is! Now when I have time tomorrow and am fresher, I'll have to see how to maybe use one of those two functions you and Dubz mentioned.


    Pardon if my brain is fading fast, but here are some ideas I had before...

    Option #1:
    During the upload, first find the current photo, and rename it to something like "delete_fa64ffcf71a3a3d754d1414ecd4d5577ab2b2fff.gif", then upload the new photo, and update the database. Then I could manually - or via a cron job - delete "old" photos.


    Option #2:
    Maybe there is a way to "query" what files are in my "uploads" directory, and see which files are "orphans"?


    Option #3:
    I could delete the current photo, then upload the new photo, and update the database. (But if I do that, I need to be damn sure that a user can only ever conceivably delete their current photo, and not someone else's photo or system files, etc. And don't just say, "Oh that can't happen?!" B.S.!!! I know enough about hackers and have read enough of the PHP manual to know there are lots of crafty ways to do evil things!!)

    This option is maybe the most obvious, BUT, I think it may be the most risky.

    So, that was the spirit of my OP, Dubz and Tango!!


    Quote Originally Posted by tangoforce View Post
    lol you're learning
    Luv ya, Tango... Glad I could provide some entertainment today!!

    Sincerely,


    Debbie

  • #8
    Master Coder felgall's Avatar
    Join Date
    Sep 2005
    Location
    Sydney, Australia
    Posts
    6,627
    Thanks
    0
    Thanked 646 Times in 636 Posts
    If you want full control over who has access to do anything with the images then store them in the database. That would remove any possibility of orphan images and would also ensure that all the images get backed up along with the rest of the database content. It is probably overkill for a simple application though since there is a space and time overhead associated with keeping them in the database. Depends on how worried you are about security of the images.
    Stephen
    Learn Modern JavaScript - http://javascriptexample.net/
    Helping others to solve their computer problem at http://www.felgall.com/

    Don't forget to start your JavaScript code with "use strict"; which makes it easier to find errors in your code.

  • #9
    Senior Coder doubledee's Avatar
    Join Date
    Mar 2011
    Location
    Arizona
    Posts
    1,071
    Thanks
    26
    Thanked 0 Times in 0 Posts
    Quote Originally Posted by felgall View Post
    If you want full control over who has access to do anything with the images then store them in the database. That would remove any possibility of orphan images and would also ensure that all the images get backed up along with the rest of the database content. It is probably overkill for a simple application though since there is a space and time overhead associated with keeping them in the database.
    Thanks for the idea, but the overwhelming consensus on this point is, "Don't do it!"

    Databases are for storing and accessing DATA.

    File systems are for storing and accessing FILES.


    Quote Originally Posted by felgall View Post
    Depends on how worried you are about security of the images.
    It's not the image files that I am worried about...

    I am worried that if I make a design mistake, then my code might allow a hacker to over-ride or trick my PHP into deleting some file other than the current member's current photo.

    Sincerely,


    Debbie

  • #10
    Regular Coder
    Join Date
    Sep 2011
    Posts
    408
    Thanks
    18
    Thanked 26 Times in 26 Posts
    Quote Originally Posted by doubledee View Post
    You are totally losing me here?!

    What does the name of a user's photo have to do with security???

    The approach I take is...
    PHP Code:
        // Hash Uploaded File.    
        
    $photoHash hash_file('sha256'$tempFileFALSE); 
    If you save images based on their username, then they have no input on what is being deleted or renamed, whether they try to inject the file name or something else. The only time this would fail is if you allow for periods and slashes in the usernames, because that could change directories and such.

    Basically it goes like this. Lets say you have /var/www/uploads/avatars as the directory you want to use with no sub directories.
    Now if I upload an image, it will save to /var/www/uploads/avatars/dubz.jpg and if tango does, it will save to /var/www/uploads/avatars/tangoforce.jpg.
    The files are named the user's username.
    Now lets say I change my avatar, and the file is named tangoforce.jpg, because lets say I wanted to put a picture of an overweight grandpa for his picture or such to tease him. Your PHP should rename the file to dubz.jpg because I uploaded it, not him. Therefore, it will save to /var/www/uploads/avatars/dubz.jpg, overwrite the old one, and my new avatar is ready.

    With this scenario, only my avatar is removed and nothing was ever deleted, only overwritten (which is still deleting technically).


    Now as for the extensions part!
    If you allow more than jpg images to be uploaded, lets say png. You will need to read the directory /var/www/uploads/avatars for any image files with the same name as the user. If you find it, then you can unlink it, then save the new one to take its place.

    Lets say I have /var/www/uploads/avatars/dubz.jpg as my avatar, but want to upload a png.
    Your code scans /var/www/uploads/avatars for any filenames with the same name as my username (/var/www/uploads/avatars/dubz.*) and delete them (be careful you check the full username and not to the first period if you allow periods in the usernames)
    Finally, my new avatar is saved to /var/www/uploads/avatars/dubz.png


    Nothing outside the directory is touched, every user has their own identifiable avatar, and as long as you don't allow slashes in usernames your directories are safe (slashes would change directories since files can't have them ).


    Actually, you could use the user's ID instead of their username, since they wouldn't have any extra periods or any slashes in them so you'd be safe with it. If you want to mask them to avoid someone spamming the site to get all the images from incrementing, you could add a suffix with a random string and save it to the database.

    Example:
    PHP Code:
    $filename $user['id'].'_'.generateRandom(32); 
    Even doing this it would be easy to delete old images, just scan for files starting with $user['id'].'_' and delete them.
    If I've helped you out, show your appreciation by clicking the "Thanks" link as well as a link below!

    AdFly
    Facebook | Twitter
    Google | YouTube

  • #11
    Senior Coder
    Join Date
    Feb 2011
    Location
    Your Monitor
    Posts
    4,313
    Thanks
    58
    Thanked 525 Times in 512 Posts
    Blog Entries
    5
    Quote Originally Posted by doubledee View Post
    Thanks for the idea, but the overwhelming consensus on this point is, "Don't do it!"

    Databases are for storing and accessing DATA.

    File systems are for storing and accessing FILES
    On the contrary, I rarely agree with felgall but he is putting forward a very good point that you should consider.

    IF you're worried about attackers, storing your files in the database is a good idea for several reasons:

    1) You can perform rollbacks - restoring old records with old files that were matching them (not possible once the files have been wiped from the file system - thus leaving you with records that have no matching files)

    2) You can the use a read / update only mysql user that has no permission to delete your records - just have a deleted column and mark that as 1 when you want to delete something instead of deleting the actual record. IF there are any weak points in your php / html code, at least attackers won't be able to start deleting stuff from the database.

    3) IF you ever need to move servers, you won't need to worry too much about file paths (yes these break quite easily - trust me) as everything will be pulled dynamically from the database.


    Don't be too quick to shoot down felgalls idea. I don't often agree with him but in this case Deb, you are wrong to assume that his idea is bad. Admittedly i don't do this often as it requires more work code wise but it does have benefits.
    Last edited by tangoforce; 06-16-2014 at 01:11 PM.
    See my new CodingForums Blog: http://www.codingforums.com/blogs/tangoforce/

    Many useful explanations and tips including: Cannot modify headers - already sent, The IE if (isset($_POST['submit'])) bug explained, unexpected T_CONSTANT_ENCAPSED_STRING, debugging tips and much more!


  •  

    Posting Permissions

    • You may not post new threads
    • You may not post replies
    • You may not post attachments
    • You may not edit your posts
    •