File upload issues in PHP

Question

Hi i'm trying to upload an image using a php script. And whats really weird is i get the following error only in Internet Explorer everywhere else script works fine:

Warning: move_uploaded_file(pictures/) [function.move-uploaded-file]: failed to open stream: Is a directory in /home/tntauto1/public_html/admin_add1.php on line 59

Warning: move_uploaded_file() [function.move-uploaded-file]: Unable to move '/tmp/phpcJnHZE' to 'pictures/' in /home/tntauto1/public_html/admin_add1.php on line 59

Warning: copy() [function.copy]: The first argument to copy() function cannot be a directory in /home/tntauto1/public_html/admin_add1.php on line 60

Here is the Script:

if(is_uploaded_file($_FILES['image']['tmp_name'])){
    if($_FILES['image']['type'] == 'image/jpeg'){
        $original = 'original_'.$v_id.'.jpg';
        $large = 'large_'.$v_id.'.jpg';
        $small = 'small_'.$v_id.'.jpg';

    }elseif($_FILES['image']['type'] == 'image/gif'){
        $original = 'original_'.$v_id.'.gif';
        $large = 'large_'.$v_id.'.gif';
        $small = 'small_'.$v_id.'.gif';
    }else{
        $error = 'Error: The image could not be uploaded. It must be in .jpg, .jpeg or .gif format.';
    }
    if(move_uploaded_file($_FILES['image']['tmp_name'],'pictures/'.$large)){}
        copy('pictures/'.$large,'pictures/'.$small);

    $imgsize = getimagesize('pictures/'.$large); //>>>>>>>>>>>>>>>>>>>>>>>>>>>>---- Resize to 480 X 360
    $width = $imgsize[0];
    $height = $imgsize[1];
    if(($width > 480) || ($height > 360)){//resize the image
        $ratio = $width / $height;
        if(100 / $ratio >= 80){//calculates if height of uploaded image is too large
            $new_width = floor(360 * $ratio);
            $new_height = 360;
        }elseif(150 * $ratio > 100){// calculate if width of uploaded image is too large
            $new_width = 480;
            $new_height = floor(480 / $ratio);
        }
        if($_FILES['image']['type'] == 'image/jpeg'){
            $img = imagecreatefromjpeg('pictures/'.$large);
            $img_copy = imagecreatetruecolor($new_width,$new_height);
            imagecopyresampled($img_copy,$img,0,0,0,0,$new_width,$new_height,$width,$height);
            imagejpeg($img_copy,'pictures/'.$large,100);    
        }
        if($_FILES['image']['type'] == 'image/gif'){
            $img = imagecreatefromjpeg('pictures/'.$large);
            $img_copy = imagecreatetruecolor($new_width,$new_height);
            imagecopyresampled($img_copy,$img,0,0,0,0,$new_width,$new_height,$width,$height);
            imagejpeg($img_copy,'pictures/'.$large,100);    
        }
    }

Hey thanks everyone for your input. Not checking it by file type $_FILES['name']['type'] fixed it. (sorry i didn't mention $large variable is defined earlier in the script that i didn't posted) — Ross, Oct 23 '09 at 18:19

score 6 · Accepted Answer · answered Oct 23 '09 at 17:53

6

if($_FILES['image']['type'] == 'image/jpeg'){

Never rely on the MIME type submitted by the browser.

In this case your problem is as david alluded to: IE usually (wrongly) supplies image/pjpeg for JPEGs, so you're detecting an unknown filetype and setting $error to Error: The image could not be uploaded. It must be in .jpg, .jpeg or .gif format.... but then despite that you still try to move the file anyway, despite not having set $small or $large.

But more than that, the browser-submitted type is likely to be completely wrong. You can't trust the uploaded filename or media type to be appropriate, so don't even bother check them. Instead, look at $imgsize[2] after your call to getimagesize to find out what type PHP thinks the image is.

And... if you are accepting image uploads from general users, you've got a security problem. It's perfectly possible to create a valid GIF (or other filetype) that contains HTML tags. Then when bloody-stupid-IE comes along to access the GIF as a page on its own it'll detect the HTML tags, decide the Content-Type you told it must be wrong, and interpret it as an HTML page instead, including any JavaScript in there, which then executes in your site's security context.

If you have to allow file uploads from an untrusted source and you're not processing the images yourself (which would usually have the side-effect of removing unwanted HTML), you generally have to serve your images from a different hostname to avoid them scripting into your site.

answered Oct 23 '09 at 17:53

bobince

528,062
107
651
834

Didn't know you could embed HTML in images. How one would do that? More importantly, how one would detect that the uploaded image contains HTML tags and process accordingly? – akinuri Aug 11 '16 at 04:27
Here is a fun example: http://lcamtuf.coredump.cx/squirrel/ . Trying to detect tags in arbitrary files is a pretty doomed approuach as you would have to use the same heuristics as browsers, which are variable and undocumented. Instead, as above, serve your user content from a different hostname so that if it gets XSS that doesn't give it control of your main site. – bobince Aug 12 '16 at 20:36
Very interesting. I can see the HTML tags when I inspect the image file. Some suggest creating a new image from the uploaded image to remove possible HTML and scripts in the file and/or in metadata. Would that work? I'm thinking about posting a question about this regarding my particular case. – akinuri Aug 12 '16 at 22:29
Recompressing the image would give you the opportunity to remove metadata, certainly. That still leaves open the possibility that the attacker sends image data that, when compressed, results in data that looks like HTML tags. (Assuming the attacker has access to the same compressor code to analyse and target,) This is an interesting but certainly difficult attack; probably impracticable for JPEG, but might be more doable for some lossless formats. – bobince Aug 15 '16 at 00:17
I have asked a [question](http://stackoverflow.com/questions/38941716/prevent-image-upload-code-injection) regarding this. You can check it out and answer it if you'd like. – akinuri Aug 15 '16 at 01:19

score 3 · Answer 2 · answered Oct 23 '09 at 17:30

if($FILES['image']['type'] == 'image/jpeg'){

Variable that holds file upload data should be $_FILES. Since $FILES is an empty (just used) variable, your $large variable is also empty so you're moving a file to the pictures/ which is a directory, just like PHP told you. Your $error should also contain the error message since none of the ifs before it is true.

One way of avoiding errors like this is to develop with error_reporting set to E_ALL that would have displayed a notice that your $FILES variable (a typo) is undefined.

Disregard this answer, it seems it only looked as if you've made a typo because of the wrong formatting of the question here. I agree with with the david.scheider's answer, check the returned mime type when uploading from IE. — Marko, Oct 23 '09 at 17:48

score 0 · Answer 3 · answered Oct 23 '09 at 17:27

0

You can't move a directory, because $large has no value, or is reset.

answered Oct 23 '09 at 17:27

CodeJoust

3,760
21
23

File upload issues in PHP

3 Answers3

Linked