I have a file upload script. Most of it was borrowed from the internet, but some of it is writing the filename to a database.
So as it is doing database work do I need to sanitize the file input?
What I’ve done so far is make the file input read-only, so people can’t edit it. But could some clever but unscrupulous person find a way to sabotage my database (or worse) using a clever file name?
Absolutely. Im not a php programmer, but I would limit the variable to contain just basic numbers/letters, dashes, no slashes, and a character limit. Im sure there’s a built in library for this.
You also want to run some kind of virus scanner on your file store. Perhaps you can put clamav on your webserver and scan your upload directly nightly. Dont let yourself become a vector for malware.
Yes. Always sanitize anything that goes into your database (or that you store then display in a browser for that matter). An unscrupulous user could insert SQL commands instead of a “filename” and wreak havoc.
I’ve sanitized it now. I found this function online. (Before your post tanstaafl)
I think I have to use this because what I’m sanitizing is a filename. And ordinary sanitization would ‘break’ the filename.
Also, I’m having doubts about going ahead with this. As HorseloverFat points out there is potential for uploading viruses. I am hosted at hostmonster.com. Anyone any experience with them?
function sanitize_filename($filename, $forceextension="")
{
/*
1. Remove leading and trailing dots
2. Remove dodgy characters from filename, including spaces and dots except last.
3. Force extension if specified
*/
$defaultfilename = "none";
$dodgychars = "[^0-9a-zA-z()_-]"; // allow only alphanumeric, underscore, parentheses and hyphen
$filename = preg_replace("/^[.]*/","",$filename); // lose any leading dots
$filename = preg_replace("/[.]*$/","",$filename); // lose any trailing dots
$filename = $filename?$filename:$defaultfilename; // if filename is blank, provide default
$lastdotpos=strrpos($filename, "."); // save last dot position
$filename = preg_replace("/$dodgychars/","_",$filename); // replace dodgy characters
$afterdot = "";
if ($lastdotpos !== false) { // Split into name and extension, if any.
$beforedot = substr($filename, 0, $lastdotpos);
if ($lastdotpos < (strlen($filename) - 1))
$afterdot = substr($filename, $lastdotpos + 1);
}
else // no extension
$beforedot = $filename;
if ($forceextension)
$filename = $beforedot . "." . $forceextension;
elseif ($afterdot)
$filename = $beforedot . "." . $afterdot;
else
$filename = $beforedot;
return $filename;
}
Are you just trying to let people upload image files? If so, the least you can do (before that cleaning script, which is cool) is don’t let anything through other than .jpg, .gif and .png. Also, don’t let the user change the filename once it’s up.
We had problems with an uploader that only accepted image files, but let the user change the filename using a filebrowser script. So they’d upload nastyscript.jpg, change it to nastyscript.php and then run it. Bad news.
Yeah I did think about limiting it to image extensions. I’ll do that if I decide to implement it.
It’s for the whatsbetter.com style page I’ve done. Wanted to make it possible for people to add their own. (moderated of course) But I’m still unsure as to the wisdom of the whole idea.
Right. I’ve sanitized it. Limited it to JPG, GIF, and JPEG, and added it to my site.
You are checking the filename for gif, jpg, jpeg? Yes?
I just uploaded a ppt file where I changed the extension to gif and it uploaded.
Baaaaaaaaaaaaaaad!
Look for a script that checks the mime type.
That way they will need to do much more futzing to get around your checking.
It’s trivially easy to change the extension on a piece of evil software.
Something like this:
$imtype = $_FILES["userfile"]["type"];
switch ($imtype)
{
case "image/pjpeg":
$filetype = ".jpg";
break;
case "image/jpeg":
$filetype = ".jpg";
break;
default:
$error_message="You are restricted to image files only.<br>Please load images with a file type of .jpg or .jpeg only";
}
I’m more concerned that he left out .PNG and other image formats, myself. In fact, what good would it do to upload a ppt as a gif (assuming the mime-type fix was in)?
ppt as gif? No good at all. It was to highlight the invalid checking that was going on.
jpg, gif, png, bmp (yuck), sure add in what needs to be done. It was an example, not an exhaustive list.
Don’t forget to check the mime type. It’s not always a straight image/whatever (i.e. image/jpeg) as you can see pjpeg is also a valid mime type.
I use something like this http://www.webmaster-toolkit.com/mime-types.shtml when I’m looking for a particular mime type to check against.
I believe you, but I am curious why. I can’t exactly figure out how a malicious ppt macro would run from within a gif. Thanks.