I've needed to create an event roster with image uploads. I've got the upload routine in place and it's working awesome on the admin side! :) But...image uploading is a touchy situation when it comes to "regular" users. So, what checks would you put in place to validate an image file and keep out the bad stuff? So far, I'm using 3 checks:

1) Check the file extension (only gif, jpg, jpeg, png allowed)
2) Check $_FILES data for "real" image using getimagesize()
3) Check filesize against admin settings

Is there anything else that should be checked against to safely allow site users to upload images? ... or will these 2 checks get the job done? (Or is there ways around these checks?)

To mention ahead of time:
1) The script resizes/resamples the image (if it's too wide or tall) so layout breakage is no concern.
2) All submissions must be approved before going live, so objectionable image content is no concern.

Forgot one other thing...

For uploading, a temp dir and the mod's image dir need CHMODing to work properly. It's quite easy to auto-CHMOD those dirs (with PHP) during the install, but would this be considered rude? Just trying to make things easier for the end user, but I haven't seen any auto-CHMOD implemented in any mods, so thought I'd ask first...

maybe have a quota for each user, like each user can only get a certain amount of space.

The submissions aren't counted toward any quota as it's not their "personal space" that they're uploading to. They'd be uploading say, a picture of Aerosmith, to go along with an Aerosmith concert.... or a pic of golf greens to go along with a golf event... That sort of thing... Basically, after approval, the image isn't attributed to the user's account...it just goes into the pool.

I appreciate your feedback. The really important thing is to not allow malicious users to upload any bad stuff to the site... I think image uploading is something that a lot of mods could use... but there needs to be a "tidy" bit of code to model after to promote the functionality. I believe I have just this code, but want to be sure it's perfectly safe all the way around. So far, so good. :)

Basically, since I plan on publicly releasing the module, I don't want to be patching it a week out. ;)

One thought on CHMOD. I was using one mod (forget which) that had the CHMODs set in such a way that I couldn't actually modify stuff that was being uploaded, nor could I modify the directory to change this. I had to go back in to the code and change all occurances of chmod so that it would work for me. (As well as getting the hosting company to delete the other files and folders.) So I'd say it's fine to chmod to 777, and maybe have an option in the mod's control panel that allows people who use the mod to change the chmod to whatever they want to use.

Standard form checks (pnSecGenAuthKey), and perhaps is_uploaded_file()?

--
itbegins.co.uk - Zikula Consulting

birtwistle.me.uk - Personal Blog


Please read the Support Guide

Ah yes, forgot about that. I've used:

if(!move_uploaded_file($_FILES['varname']['tmp_name']['varname'], $upload_file)) { ... }

Does this kill the two birds with one stone in this case?

I've stripped the AuthKey functions for the moment...it's much easier to refresh than to re-fill out a 20-input form! ;) (I'm implementing them for release though.)