Image upload form trouble

bagpuss03

Hi,

A while ago I received lots of help setting up a php form with allowed an image to be attached to the email via a web form (http://www.phpbuilder.com/board/showthread.php?t=10331248). Thanks 🙂

It works great when you want to upload an image. However, I have just noticed that the form is not sent and the viewer is redirected to the forbidden page when you do NOT add an image. I have tried removing the whole section of the script which deals with the images to test that this is actually what is causing the trouble and it worked fine, so the problem obviously lies in this bit of script:

$allowed_types = array( // List all allowed MIME Types here
                    'image/gif',
					'image/pjpeg',
                    'image/jpeg',
                    'image/jpg',
                    'image/tiff',
                    'image/bmp',
                    'image/png',
                );

$extArray = array('jpg', 'jpeg', 'jpe', 'gif', 'tif', 'tiff', 'png', 'bmp'); 

if ( is_uploaded_file($_FILES['userfile']['tmp_name']) && in_array($_FILES['userfile']['type'], $allowed_types) && in_array(substr($_FILES['userfile']['name'], strrpos($_FILES['userfile']['name'], '.')+1), $extArray )) {

$fileName = $_FILES['userfile']['tmp_name'];
$fileHandle = fopen($fileName, 'r');
$fileAttach = fread($fileHandle, filesize ($fileName));
fclose($fileHandle);

$fileAttach = chunk_split(base64_encode($fileAttach));

$emailBody .= "--FTG_BOUNDRY\n"
  . "Content-Type: " . $_FILES['userfile']['type'] . "; name=\"" . $_FILES['userfile']['name'] . "\"\n"
  . "Content-disposition: attachment\n"
  . "Content-transfer-encoding: base64\n"
  . "\n"
  . "$fileAttach\n"
  . "\n";
} else {

header("Location: http://www.mysite.com/forbidden.html");
  exit;
}

All I can think is that possibly it's because the script says "if there's an image which complies with these restrictions, then send it , if not exit and redirect to ...forbidden". Was I so wrapped up in making sure only certain files were sent that I forgot to make it ok not to send an image at all?

If so, how would I rectify the script? (Please spell it out to me, cos my PHP knowledge is basic!).

I will look forward to a reply, since this seems to be a pretty stupid mistake that needs sorting quick!! :eek:

crux_op

if ( is_uploaded_file($_FILES['userfile']['tmp_name']) && in_array($_FILES['userfile']['type'], $allowed_types) && in_array(substr($_FILES['userfile']['name'], strrpos($_FILES['userfile']['name'], '.')+1), $extArray )) {

That bit above checks:
Is this an uploaded file?
Is this file in the allowed types?
Does it have an allowed extension.

If any of those checks fails.. it sends you to the forbidden zone. You want to make it skip this entire checking sequence if there's no file.

So wrap the entire code block (everything you posted), in something like this:

if( isset($_FILES['userfile']['tmp_name'])  && count($_FILES) > 0)
{
<!---- YOUR CODE HERE --- >
}

bagpuss03

Thanks for the reply. No joy I'm afraid. I would have thought it made perfect sense, since if I delete that section of code, the script works fine and the code you gave me effectively says skip this if there's no file uploaded.

Any more suggestions? Why wouldn't this work? I could post the whole script if it would help.

bagpuss03

Actually, I think I've figured it out now after playing around a bit.

I went along the same principal as you suggested, so that the script doesn't exit just because there is no image. I looked at how the script was originally, before I added bits and pieces with the help of this forum to limit the file type and size that could be uploaded. It basically said:

if ( is_uploaded_file($_FILES['userfile']['tmp_name']) ) { 

$fileName = $_FILES['userfile']['tmp_name']; 
$fileHandle = fopen($fileName, 'r'); 
$fileAttach = fread($fileHandle, filesize ($fileName)); 
fclose($fileHandle); 

$fileAttach = chunk_split(base64_encode($fileAttach)); 

$emailBody .= "--FTG_BOUNDRY\n" 
  . "Content-Type: " . $_FILES['userfile']['type'] . "; name=\"" . $_FILES['userfile']['name'] . "\"\n" 
  . "Content-disposition: attachment\n" 
  . "Content-transfer-encoding: base64\n" 
  . "\n" 
  . "$fileAttach\n" 
  . "\n"; 
}

Of course the way I had edited this for security meant that the script exited instead of just carrying on if there was no file uploaded. It's really all my own fault because I entered the exit & redirect bit without guidance to let the user know they had done wrong, thinking I was being clever!

With this new bit I simply re-entered the original if statement and put my new one withinin it to carry out further checks before proceeding....

$allowed_types = array( // List all allowed MIME Types here
                    'image/gif',
					'image/pjpeg',
                    'image/jpeg',
                    'image/jpg',
                    'image/tiff',
                    'image/bmp',
                    'image/png',
                );

$extArray = array('jpg', 'jpeg', 'jpe', 'gif', 'tif', 'tiff', 'png', 'bmp'); 

if( is_uploaded_file($_FILES['userfile']['tmp_name']) ) 
{
if ( is_uploaded_file($_FILES['userfile']['tmp_name']) && in_array($_FILES['userfile']['type'], $allowed_types) && in_array(substr($_FILES['userfile']['name'], strrpos($_FILES['userfile']['name'], '.')+1), $extArray )) {

 $fileName = $_FILES['userfile']['tmp_name'];
 $fileHandle = fopen($fileName, 'r');
 $fileAttach = fread($fileHandle, filesize ($fileName));
 fclose($fileHandle);

 $fileAttach = chunk_split(base64_encode($fileAttach));

 $emailBody .= "--FTG_BOUNDRY\n"
  . "Content-Type: " . $_FILES['userfile']['type'] . "; name=\"" . $_FILES['userfile']['name'] . "\"\n"
  . "Content-disposition: attachment\n"
  . "Content-transfer-encoding: base64\n"
  . "\n"
  . "$fileAttach\n"
  . "\n";
} else {

header("Location: http://www.kraftworkvehiclerefinishing.com/forbidden.html");
  exit;
}
}

...well, it works but I would like your advice on whether I have made the script too messy or made the script unsecure again as far as image uploading goes.