Upload issue when image exceeds php ini limit

[Drongo_III]

Hi Guys

 

Probably a simple answer here that i'm missing.

 

I've got a simple upload script for an image uploader that does various checks for security sake.

 

One of the checks is on file size to make sure it doesn't exceed the upload limit set in php.ini . The limit is set at 2mb, so the script checks that the file doesn't exceed this limit.

 

The problem occurs when I try to upload a file that far exceeds the limit (the one below is 3.5mb). When i do this the upload gets passed my script because the $_FILES['picture1']['size'] gets set to no value. A print_r of the files array shows:

 

[picture1] => Array
        (
            [name] => DSC01468.JPG
            [type] => 
            [tmp_name] => 
            [error] => 1
            [size] => 0
        )

 

So what i wanted to know was whether this was normal?

 

And should i simply do a check to see if the size value is empty?

 

Any advice would be good!

 

Thanks Drongo

[PFMaBiSmAd]

You should check the ['error'] element for that particular problem. http://www.php.net/manual/en/features.file-upload.errors.php

 

Note, exceeding the post_max_size setting will result in both the $_POST and $_FILES arrays being empty. Also an invalid form, a form with no type='file' field, and uploads not enabled on the server will result in an empty $_FILES array. You can get the actual size of the uploaded file in $_SERVER['CONTENT_LENGTH']

[Drongo_III]

Of course! I completely forgot to check errors... 

 

Thanks for the help PFMaBiSmAd - you are teaching me a lot.

 

 

You should check the ['error] element for that particular problem. http://www.php.net/manual/en/features.file-upload.errors.php

 

Note, exceeding the post_max_size setting will result in both the $_POST and $_FILES arrays being empty. Also an invalid form, a form with no type='file' field, and uploads not enabled on the server will result in an empty $_FILES array. You can get the actual size of the uploaded file by testing $_SERVER['CONTENT_LENGTH']

[Drongo_III]

Hi PFMaBiSmAd

 

I thought checking the error code would solve the issue but for some reason when the file far exceeds the php.ini limit it just seems to bypass the script.

 

The code below works fine if I upload an image that's within the upload limit but it won't work if the file exceeds the upload limit.

 

Any ideas why that might be?

 

 

 

if($value['error'] == 1)
				{
					return "The file you've attempted to upload exceeds the server limit";
				}

 

Incidentally the reason the array is $value['error'] is because its part of a loop on the files array.

 

 

You should check the ['error'] element for that particular problem. http://www.php.net/manual/en/features.file-upload.errors.php

 

Note, exceeding the post_max_size setting will result in both the $_POST and $_FILES arrays being empty. Also an invalid form, a form with no type='file' field, and uploads not enabled on the server will result in an empty $_FILES array. You can get the actual size of the uploaded file in $_SERVER['CONTENT_LENGTH']

[xyph]

Explain 'bypass.' It's definitely used incorrectly in this scope.

 

It would blow my mind if PHP wasn't running out of memory, sometimes an error message is generated, other times a 500 server error.

 

What is $_SERVER['CONTENT_LENGTH'] returning?

[PFMaBiSmAd]

As already stated, there is no $_FILES array when you exceed the post_max_size setting. There's nothing to loop over (you are likely getting php error messages when you do try.) You need to test for the existence of the $_FILES array and/or test the $_SERVER['CONTENT_LENGTH'] against the post_max_size value.

 

[Drongo_III]

Well when i say "bypassing"as it seems as though it is, because if i upload a file within the upload limit and set the code to 0 it returns the statement.

 

content_length is returning - 3498774 - i.e. the file size of the large file.

 

What am i doing wrong?

 

 

Explain 'bypass.' It's definitely used incorrectly in this scope.

 

It would blow my mind if PHP wasn't running out of memory, sometimes an error message is generated, other times a 500 server error.

 

What is $_SERVER['CONTENT_LENGTH'] returning?

[Drongo_III]

Stick with me on this one and i apologise if i'm being slow on the uptake.

 

It occurs to me that the $_FILES array is being set because when i print_r of $_FILES after i upload i see the array in my original post -i.e. listing the file i uploaded. So i might be misunderstanding you but doesn;t that signify the array is set?

 

The only issue with testing  $_SERVER['CONTENT_LENGTH'] is that I have three upload fields - so the user could legitimately exceed the limit - i.e. if i just trest content_length against 2mb

 

 

 

As already stated, there is no $_FILES array when you exceed the post_max_size setting. There's nothing to loop over (you are likely getting php error messages when you do try.) You need to test for the existence of the $_FILES array and/or test the $_SERVER['CONTENT_LENGTH'] against the post_max_size value.

[xyph]

post_max_size would include all 3 files as well, so I don't understand your argument.

 

Your $_FILES array may not be empty, but it's returning ['error'] as 1

 

UPLOAD_ERR_INI_SIZE

Value: 1; The uploaded file exceeds the upload_max_filesize directive in php.ini.

 

You still haven't explained what you mean by bypass, at least using standard terminology that we can understand. Set what code to zero? Why are you setting codes? You should be reading them.

 

if( !isset($_FILES['yourInput']) || !empty($_FILES['yourInput']['error']) ) {
echo 'There was something wrong uploading';
} else {
echo 'The upload went fine';
}

 

Should work fine for checking if it's uploaded.

 

Why is getting the size of the over-sized file important?

[Drongo_III]

Hi xyph

 

I wasn't necessarily trying to argue that my point was correct...just trying to convey my understanding of it so you can set me straight as i am clearly doing something wrong.

 

I wasn't changing codes per se. What i meant was if i do:

 

if($value['error'] == 0)
				{
					return "This triggered the return";
				}

 

and then upload a file that is within the upload limit, as set in php.ini, then i get the statement returned - indicating the code works.

 

But if i do:

if($value['error'] == 1)
				{
					return "This triggered the return";
				}

 

this time uploading the big image file, i get nothing returned. Even though a print_r of files shows that this large file has an error code of 1.

 

So to me it seemed that based on that test,  scenario two meant that when i uploaded a large image file it bypassed this condition somehow. But that might have been a poor choice of words.

 

I will try your code out now and thank you for your patience

 

 

 

 

 

post_max_size would include all 3 files as well, so I don't understand your argument.

 

Your $_FILES array may not be empty, but it's returning ['error'] as 1

 

UPLOAD_ERR_INI_SIZE

Value: 1; The uploaded file exceeds the upload_max_filesize directive in php.ini.

 

You still haven't explained what you mean by bypass, at least using standard terminology that we can understand. Set what code to zero? Why are you setting codes? You should be reading them.

 

if( !isset($_FILES['yourInput']) || !empty($_FILES['yourInput']['error']) ) {
echo 'There was something wrong uploading';
} else {
echo 'The upload went fine';
}

 

Should work fine for checking if it's uploaded.

 

Why is getting the size of the over-sized file important?

[xyph]

Try bypassing whatever you're doing to convert it to $value['error'] and instead use the $_FILES values directly. This will help isolate the issue.

[Drongo_III]

That was a good call. It seems outside of my loop, and directly accessing the $_FILES array, it worked fine :/

 

Ok you're probably going to slam my poor coding now but here is the loop that checks the upload...its my first shot at trying to make an uploader that's secure so go easy hehe...

 

The line thats causing the issue is around line 50. Have i just done this all wrong?

 

	public function imgcheck(){	

	 $test = array();				

			 foreach($_FILES as $key => $value){




			 if(!empty($value['tmp_name'])){			 


					 //Check that the file is an upload

					if(!is_uploaded_file($value['tmp_name'] )){

						return "This file has not been uploaded. You cannot do this.";

					}

					//Check if we're dealing with real images		

					if(!getimagesize($value['tmp_name'])){

						return "The image you uploaed for " . $key  . "is not a valid image type";
					}


					// CHECK FILE TYPE IS VALID

					 $allowed_types = array("image/gif","image/jpeg","image/pjpeg"); 

					if(!in_array($value["type"],$allowed_types)) { 

					return "You have attempted to upload an unsupported file type. The system only accepts JPEG and GIF files" . $key . " " . $value['size'] . " " . $value['error'] ;

					}

						// THIS IS THE CODE CAUSING ME AN ISSUE


					if($value['error'] == 1)
					{
						return "The file you've attempted to upload exceeds the server limit";
					}



					if($value['size'] > 2097140)
					{

						return "The file you're attempting to upload exceeds the maximum file limit of 2MB";

					}


				}

				else{

					return "clean";}
	 }

 

Try bypassing whatever you're doing to convert it to $value['error'] and instead use the $_FILES values directly. This will help isolate the issue.

[xyph]

Your first check is

 

!empty($value['tmp_name'])

 

So, if it's empty (which it is in your example), your function will return 'clean'

[Drongo_III]

Oh i am a total prat!

 

I have just realised the issue...

 

 if(!empty($value['tmp_name'])){			

 

That's the first statement evaluated...and obviously a file that exceeds the limit doesn't get tmp_name set... I am a donkey.

 

Apologies for being a waste of time guys.

 

Out of interest though is my code on the road to being fairly secure?

[Drongo_III]

yeah sorry for taking you round the houses...i think i need a few more early nights.

 

Besides having a turnip for a brain...is that code sort of on the right road to being secure?

 

Your first check is

 

!empty($value['tmp_name'])

 

So, if it's empty (which it is in your example), your function will return 'clean'

[xyph]

Looks okay.

 

The most important part is when moving the file to it's final destination

 

You want to set the permissions to the minimum required

http://stackoverflow.com/questions/290483/what-file-permissions-should-i-set-for-uploaded-files

 

If you're using the $_FILES['fileInput']['name'] value at all, strip any bad characters out. Slashes, line-breaks, anything that shouldn't be in a filename. This is a pretty decent RegEx for that:

$fileName = preg_replace('/[^\w\._]+/', '_', $fileName);

 

It converts anything that isn't a word character, fullstop or underscore to an underscore.

 

There might be more, but that covers the basics.

[Drongo_III]

Cool thanks for that Xyph. Good know i'm on the right track.

 

i wast planning on concatenating time() and mt_rand() to produce random file name for when i move the file.

 

When you talk about the destination, what's your opinion on the need to store the file outside the document root?

 

I was planning to just store the files in the doc root but then place .htaccess file to stop execution of any scripts. That sound about right? it's purely an iamge directory.

 

Looks okay.

 

The most important part is when moving the file to it's final destination

 

You want to set the permissions to the minimum required

http://stackoverflow.com/questions/290483/what-file-permissions-should-i-set-for-uploaded-files

 

If you're using the $_FILES['fileInput']['name'] value at all, strip any bad characters out. Slashes, line-breaks, anything that shouldn't be in a filename. This is a pretty decent RegEx for that:

$fileName = preg_replace('/[^\w\._]+/', '_', $fileName);

 

It converts anything that isn't a word character, fullstop or underscore to an underscore.

 

There might be more, but that covers the basics.

[xyph]

IMO it's not a big deal if you have checks in place. If the file can't be executed, even a script making it to web root shouldn't do damage.

 

It's a solid extra layer of security though, so I don't think it should be dismissed completely.

[El Chupacodra]

It's not a complete time waster - it's always informative to read threads like this.

But if you don't want to waste any more time you could update your thread and click solved.

[Drongo_III]

thanks for all your help Xyph

 

IMO it's not a big deal if you have checks in place. If the file can't be executed, even a script making it to web root shouldn't do damage.

 

It's a solid extra layer of security though, so I don't think it should be dismissed completely.