Be a Supporter!

PHP: securing file uploads

  • 2,139 Views
  • 8 Replies
New Topic Respond to this Topic
Pilot-Doofy
Pilot-Doofy
  • Member since: Sep. 13, 2003
  • Offline.
Forum Stats
Member
Level 37
Musician
PHP: securing file uploads Nov. 5th, 2005 @ 09:17 AM Reply

Note: If you have not read the first tutorial and do not know how to upload files with php, please visit the PHP: file uploads tutorial I have written as well.

When letting a user upload a file, it is a very bad idea to not run any checks on that file. In the previous tutorial I showed you how to run a few cosmetic checks on it, but nothing that REALLY prevented someone from messing with your server.

Sure, stopping invalid mime types, excessive filesizes, and blank files could decrease the amount of spam by a lot, but it isn't enough to stop those who really know how to goof things up.

In this tutorial I will only discuss functions which are predefined in php's standard library for us; however, those are definitely not the only checks you're capable of executing. Firstly, do you remember the move_uploaded_file() and/or copy() functions? Well, how can you ensure that the file that's really being copied is the one that was selected by the file upload field?

If a user inputs something that has special meaning to the server, for instance, /../index.html as the file name, it could overwrite necessary elements to your website. PHP has a predefined function which helps us minimize this problem fairly well, it's called is_uploaded_file() and it takes one argument which is a string and is the file name you want to check for.

If you are running a version of php that is older than 4.0.3 then you may need to create or redefine the function to use yourself, because it is pretty useful. Here is a sample version which could be used for php versions less than 4.0.3.

function is_uploaded_file($filename)
{
if (!$tmp_file = get_cfg_var('upload_tmp_dir')) {
$tmp_file = dirname(tempnam('', ''));
}
$tmp_file .= '/' . basename($filename);
/* User might have trailing slash in php.ini... */
return (ereg_replace('/+', '/', $tmp_file) == $filename);
}

# Here is an example of the self-defined function in action, it's slightly different
if (is_uploaded_file($HTTP_POST_FILES['userfi
le'])) {
copy($HTTP_POST_FILES['userfile'], "/place/to/put/uploaded/file");
} else {
echo "Possible file upload attack: filename '$HTTP_POST_FILES[userfile]'.";
}

Next, we can check file extensions as well. We can check file extensions to make sure the user didn't simply spoof the mimetype. Now, of course there are ways of spoofing both the mimetype and file extension, but I'm lucky enough to not have seen much of that in my day.

We could use a simply regular expression in order to check for file extensions we want to allow. File extensions can be found in the name element of the $_FILES superglobal. Let's say we wanted to allow users to upload pictures for a photo album, but we only wanted to allow .gif, .jpeg, and .png extensions. Below is an example of the code:

# Other file upload code above this
$allowed_filetypes = array('gif', 'jpeg', 'png', 'jpg');
# You should only have to edit the line above

$preg_filetypes = join('|', $allowed_filetypes);
if ( !preg_match('#.*?\.(' . $preg_filetypes . ')#si', $_FILES['data']['name']) ) {
# Invalid file extension
die('Invalid file extension. Only the following are allowed: ' . join(', ' , $allowed_filetypes));
}

$match = false;
foreach($allowed_filetypes as $type) {
if ($_FILES['data']['type'] == 'image/' . $type) {
$match = true;
}
} // End foreach

if ($match !== true) {
die('Invalid mimetype for your file.');
}

Note, if you are using the example for upload.php that was found in the other tutorial linked at the top of the page, you should delete lines 16-19 and place this code there instead.

With those steps, you can help secure your php file uploads, but those aren't the only precausions you can take to ensure safety for your server, website, and other users.

henke37
henke37
  • Member since: Sep. 10, 2004
  • Offline.
Forum Stats
Member
Level 30
Blank Slate
Response to PHP: securing file uploads Nov. 5th, 2005 @ 11:28 AM Reply

Sorry, your code is insecure. Again.
The ereg fails to check that the extension is at the end of the name. A hacker only has to upload "loser.gif.php" with the mime type "image/gif".


Each time someone abuses hittest, God kills a kitten. Please, learn real collision testing.

DFox
DFox
  • Member since: Aug. 9, 2003
  • Offline.
Forum Stats
Member
Level 30
Blank Slate
Response to PHP: securing file uploads Nov. 5th, 2005 @ 12:19 PM Reply

At 11/5/05 11:28 AM, henke37 wrote: Sorry, your code is insecure. Again.
The ereg fails to check that the extension is at the end of the name. A hacker only has to upload "loser.gif.php" with the mime type "image/gif".

I think you're wrong. I am not that good with regular expressions, but I believe when he is checking the extensions with the expression, he is saying it has to be at the end, with nothing following.

So I think it is secure.


BBS Signature
Afro-Ninja
Afro-Ninja
  • Member since: Mar. 2, 2002
  • Offline.
Forum Stats
Moderator
Level 44
Game Developer
Response to PHP: securing file uploads Nov. 5th, 2005 @ 01:34 PM Reply

hmm. It just needs a dollar sign to anchor it to the end, right?


BBS Signature
WoogieNoogie
WoogieNoogie
  • Member since: Jun. 26, 2005
  • Offline.
Forum Stats
Supporter
Level 15
Programmer
Response to PHP: securing file uploads Nov. 5th, 2005 @ 02:00 PM Reply

Hah, I love these PHP posts. Very helpful.

Pilot-Doofy
Pilot-Doofy
  • Member since: Sep. 13, 2003
  • Offline.
Forum Stats
Member
Level 37
Musician
Response to PHP: securing file uploads Nov. 5th, 2005 @ 03:47 PM Reply

At 11/5/05 01:34 PM, Afro_Ninja wrote: hmm. It just needs a dollar sign to anchor it to the end, right?

Yes, sorry, I forgot to add that in. Simple mistake. So, here is the quickest fix which handles it:

Change this:
if ( !preg_match('#.*?\.(' . $preg_filetypes . ')#si', $_FILES['data']['name']) ) {
to this:
if ( !preg_match('#.*?\.(' . $preg_filetypes . ')$#si', $_FILES['data']['name']) ) {

Pilot-Doofy
Pilot-Doofy
  • Member since: Sep. 13, 2003
  • Offline.
Forum Stats
Member
Level 37
Musician
Response to PHP: securing file uploads Nov. 5th, 2005 @ 03:52 PM Reply

Bah, sorry for the double post, but I should remind everyone (and Henke) that you should also check for extensions hidden away that Apache doesn't reconize.

I was just talking to Sir-Davey about this (F-13 lol I made up his name, isn't it hawt?) and he told me I should probably include the fact that if you're checking for uncommon file types such as .fla which Apache doesn't reconize, you might want to check for other extensions hidden away such as .php, .phtml, .php3, etc.

Mainly because to apache page.php.fla will be parsed as page.php since Apache doesn't reconize the Macromedia .fla extension.

Just thought I'd throw that out there.

Afro-Ninja
Afro-Ninja
  • Member since: Mar. 2, 2002
  • Offline.
Forum Stats
Moderator
Level 44
Game Developer
Response to PHP: securing file uploads Nov. 16th, 2005 @ 10:13 PM Reply

At 11/5/05 09:17 AM, Pilot-Doofy wrote: $match = false;
foreach($allowed_filetypes as $type) {
if ($_FILES['data']['type'] == 'image/' . $type) {
$match = true;
}
} // End foreach

if ($match !== true) {
die('Invalid mimetype for your file.');
}

instead of doing that I just have
$type=$_FILES['data']['type'];
if($type != "image/gif" && $type != "image/pjpeg") showMessage('<span class="leadText">Invalid file extension. Images must be in gif or jpg format only</span>');

and then I have the preg match for jpg|jpeg|gif

That should be pretty secure right? Is it still possible for someone to get a php script through that?


BBS Signature
Pilot-Doofy
Pilot-Doofy
  • Member since: Sep. 13, 2003
  • Offline.
Forum Stats
Member
Level 37
Musician
Response to PHP: securing file uploads Nov. 16th, 2005 @ 10:15 PM Reply

At 11/16/05 10:13 PM, Afro_Ninja wrote: That should be pretty secure right? Is it still possible for someone to get a php script through that?

Yes, checking both is a nice added security, even if checking mime types does a minimal amount of good. Never say never, but most of the idiots won't know how to get past that.