tags:

views:

71

answers:

4

I found this snippet that says will only allow certain file types. Will it work and could someone bypass it to upload what ever file type they want? And could someone explain the substr part, i don't get how it works..

<?php
function CheckExt($filename, $ext) {
    $name = strtolower($filename);
    if(substr($name, strlen($name) -3, 3) == $ext)
        return true;
    else
        return false;
}
?>
A: 

People will still be able to upload whatever they want; they just have to give the file a particular extension.

For substr, see the manual.

Artefacto
+5  A: 

It's very easy to bypass as changing the extension of a file does not change the contents of the file. So a .exe renamed into a .jpg is still an .exe waiting to be run anyway. You can use it for a basic check, but don't rely solely on it to validate file types.

This substr() call:

substr($name, strlen($name) -3, 3)

Is better more simply written as:

substr($name, -3)

Which PHP just interprets as 'take only the last 3 characters of $name'.

EDIT: it's not better per se because file extensions don't necessarily have to be 3 characters long. They could be 2, they could be 4, 5, even 10. This is why as I said, checking file extensions isn't very reliable.

BoltClock
ok thanks, but I mean I don't want people uploading shells, if they change it to an image extension, it they would not be able to execute the php, or could they?
Dr Hydralisk
@Dr Hydralisk when allowing any users to upload files you should be setting the permissions of those files to be non-executable!!
Ben Rowe
@Dr Hydralisk: well they probably wouldn't be able to, but it's best to be safe I guess. No point keeping a malicious file either way.
BoltClock
+7  A: 

A better way to check extensions

function checkExt($filename, $ext)
{
  $fnExt = strtolower(pathinfo($filename, PATHINFO_EXTENSION));
  if(!is_array($ext)) {
    $ext = (array)$ext;
  }
  $ext = array_map('strtolower', $ext);
  return in_array($fnExt, $ext);
}

You can then call it like

var_dump(checkExt('test.temp', 'tmp')); // false
var_dump(checkExt('test.temp', array('tmp', 'temp'))); // true

Avoid using substr as the extension length is unknown (you can use substr & strrpos as well but php provides this functionality for you)

Ben Rowe
+1  A: 

I prefer to whitelist the Mimetypes I want to allow using something along the lines of

 $mimesGeneral = array(
        'txt'=>'text/plain',
        'doc'=>'application/msword',
        'pdf'=>'application/pdf',
        'xls'=>'application/x-excel',
        'xls'=>'application/excel',
        'xls'=>'application/vnd.ms-excel',
        'rtf'=>'application/rtf',
        'zip'=>'application/zip'

        );
$success = false;
foreach($allowedMimes as $key=>$value){

            if($_FILES['uploaded_file']['type'] == $value){

                return true;
            }
        }

I use this with a blacklist of suffixes e.g 'php', 'pl', 'exe' etc...

niggles
mimetypes can also be faked as well as extensions.
Ben Rowe
didn't know that -> except for appending data onto an image which is why I always run user input through GD and create a new image rather than use the raw one.I guess this is another area to look into then for anything that allows user upload.
niggles