# PHP do I need to sanitize a file uploader?

**URL:** <https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477>\
**Category:** Factual Questions\
**Created:** [September 9, 2009, 9:20pm UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477 "2009-09-09T21:20:31Z")\
**Posts on this page:** 12\
**Page:** 1

<div class="post-metadata">

**Author:** ![Lobsang](https://sea3.discourse-cdn.com/straightdope/user_avatar/boards.straightdope.com/lobsang/32/4067_2.png) [@Lobsang](https://boards.straightdope.com/u/Lobsang)\
**Post date:** [September 9, 2009, 9:20pm UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/1 "2009-09-09T21:20:31Z")

</div>

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?

---

<div class="post-metadata">

**Author:** ![HorseloverFat](https://avatars.discourse-cdn.com/v4/letter/h/8e8cbc/32.png) [@HorseloverFat](https://boards.straightdope.com/u/HorseloverFat)\
**Post date:** [September 9, 2009, 9:27pm UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/2 "2009-09-09T21:27:41Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![HorseloverFat](https://avatars.discourse-cdn.com/v4/letter/h/8e8cbc/32.png) [@HorseloverFat](https://boards.straightdope.com/u/HorseloverFat)\
**Post date:** [September 9, 2009, 9:54pm UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/3 "2009-09-09T21:54:35Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![tanstaafl](https://avatars.discourse-cdn.com/v4/letter/t/ba8739/32.png) [@tanstaafl](https://boards.straightdope.com/u/tanstaafl)\
**Post date:** [September 9, 2009, 10:51pm UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/4 "2009-09-09T22:51:30Z")

</div>

[Yes](http://xkcd.com/327/). 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.

[Here’s a good, quick tutorial on what to do.](http://www.denhamcoote.com/php-howto-sanitize-database-inputs)

---

<div class="post-metadata">

**Author:** ![Lobsang](https://sea3.discourse-cdn.com/straightdope/user_avatar/boards.straightdope.com/lobsang/32/4067_2.png) [@Lobsang](https://boards.straightdope.com/u/Lobsang)\
**Post date:** [September 9, 2009, 11:45pm UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/5 "2009-09-09T23:45:49Z")

</div>

> [@tanstaafl](#):
>
> [Yes](http://xkcd.com/327/). 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.
> 
> [Here’s a good, quick tutorial on what to do.](http://www.denhamcoote.com/php-howto-sanitize-database-inputs)

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](http://hostmonster.com). Anyone any experience with them?

```php
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;
}

```

---

<div class="post-metadata">

**Author:** ![ZipperJJ](https://sea3.discourse-cdn.com/straightdope/user_avatar/boards.straightdope.com/zipperjj/32/211_2.png) [@ZipperJJ](https://boards.straightdope.com/u/ZipperJJ)\
**Post date:** [September 10, 2009, 1:47am UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/6 "2009-09-10T01:47:41Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![Lobsang](https://sea3.discourse-cdn.com/straightdope/user_avatar/boards.straightdope.com/lobsang/32/4067_2.png) [@Lobsang](https://boards.straightdope.com/u/Lobsang)\
**Post date:** [September 10, 2009, 9:58pm UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/7 "2009-09-10T21:58:59Z")

</div>

> [@ZipperJJ](#):
>
> 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](http://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.

---

<div class="post-metadata">

**Author:** ![Lobsang](https://sea3.discourse-cdn.com/straightdope/user_avatar/boards.straightdope.com/lobsang/32/4067_2.png) [@Lobsang](https://boards.straightdope.com/u/Lobsang)\
**Post date:** [September 11, 2009, 12:44am UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/8 "2009-09-11T00:44:48Z")

</div>

Right. I’ve sanitized it. Limited it to JPG, GIF, and JPEG, and added it to my site.

[http://notails.com](http://notails.com).

---

<div class="post-metadata">

**Author:** ![Caught\_Work](https://avatars.discourse-cdn.com/v4/letter/c/919ad9/32.png) [@Caught\_Work](https://boards.straightdope.com/u/Caught_Work)\
**Post date:** [September 11, 2009, 11:52am UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/9 "2009-09-11T11:52:49Z")

</div>

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:

```auto

         $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";
              }

```

---

<div class="post-metadata">

**Author:** ![BigT](https://sea3.discourse-cdn.com/straightdope/user_avatar/boards.straightdope.com/bigt/32/12044_2.png) [@BigT](https://boards.straightdope.com/u/BigT)\
**Post date:** [September 15, 2009, 10:52pm UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/10 "2009-09-15T22:52:18Z")

</div>

> [@Caught\_Work](#):
>
> 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:
> 
> ```auto
> 
> $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)?

---

<div class="post-metadata">

**Author:** ![Caught\_Work](https://avatars.discourse-cdn.com/v4/letter/c/919ad9/32.png) [@Caught\_Work](https://boards.straightdope.com/u/Caught_Work)\
**Post date:** [September 16, 2009, 12:36pm UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/11 "2009-09-16T12:36:04Z")

</div>

> [@BigT](#):
>
> 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](http://www.webmaster-toolkit.com/mime-types.shtml) when I’m looking for a particular mime type to check against.

---

<div class="post-metadata">

**Author:** ![StinkyBurrito](https://avatars.discourse-cdn.com/v4/letter/s/b9e5f3/32.png) [@StinkyBurrito](https://boards.straightdope.com/u/StinkyBurrito)\
**Post date:** [September 16, 2009, 12:52pm UTC](https://boards.straightdope.com/t/php-do-i-need-to-sanitize-a-file-uploader/509477/12 "2009-09-16T12:52:09Z")

</div>

> [@Caught\_Work](#):
>
> ppt as gif? No good at all.

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.