File upload issues

[sford999]

Hi,

I have this function to upload files via an online form

function do_upload()
{
	if(!$_COOKIE['username'] && !$_COOKIE['password'])
	{
		make_header($title = 'Error: You must be logged in to upload files');
		echo "<p>Error: Sorry, you must be logged in to upload files.</p>";
		make_footer();
		exit();
	}
	else
	{
		make_header($title = 'Upload Images');
		//Set the maximum file size in bytes
		$max_file_size = 153600;
		// Set the max width in pixels
		$max_width = 800;
		// Set the max height in pixels
		$max_height = 600;
		// Set the directory to upload to
		$directory_to_upload = UPLOAD_PATH;
		$mfs = $max_file_size / 1024;


	if(isset($_POST['submit']))
	{
		// get the time for a random image name
		$time = time();
		$image_dimensions = getimagesize($_FILES['image_to_upload']['tmp_name']);
		$image_height = $image_dimensions[1];
		$image_width = $image_dimensions[0];
		$errors = array();
		if(!$_FILES['image_to_upload']['name'])
		{
			$errors[] = "You did not select a file to upload.<br />";
		}

		if(!is_uploaded_file($_FILES['image_to_upload']['tmp_name']))
		{
			$errors[] = "Error while uploading file.<br />";
			make_footer();
			exit();
		}

		if($_FILES['image_to_upload']['size'] > $max_file_size)
		{
			$errors[] = "Your image size was too big. The maximum filesize is $mfs Kilobytes.<br />";
		}
		// check the image type, gif or jpg/jpeg is allowed
		if(!exif_imagetype($_FILES['image_to_upload']['tmp_name']) == IMAGETYPE_GIF || !exif_imagetype($_FILES['image_to_upload']['tmp_name']) == IMAGETYPE_JPEG || !exif_imagetype($_FILES['image_to_upload']['tmp_name']) == IMAGETYPE_PNG)
		{
			$errors[] = "Wrong file type, you can only upload .jpg, .gif and .png files.<br />";
		}
		// check the iamge dimensions
		if($image_height > $max_width)
		{
			$errors[] = "Your image height was too large.<br />";
		}

		if($image_width > $max_width)
		{
			$errors[] = "Your image width was too large.<br />";
		}
		if(count($errors) > 0)
		{
			echo "Error: ";
			foreach($errors as $err)
			{
				echo $err . "<br />";
				make_footer();
				exit();
			}
		}
		else
		{
			$comment = mysql_real_escape_string(htmlspecialchars(addslashes($_POST['comment'])));
			$user_id = base64_decode($_COOKIE['user_id']);
			$image_url = $directory_to_upload . $user_id . '-' . $time . '-' . $_FILES['image_to_upload']['name'];
			$img_id = $user_id . '-' . $time . '-' . $_FILES['image_to_upload']['name'];
			if(copy($_FILES['image_to_upload']['tmp_name'], $image_url))
			{
				db_connect();
				$sql = "INSERT INTO images (img_id, user_id, comments, date, auth) VALUES ('$img_id', '$user_id', '$comment', '$time', '0')";
				$result = mysql_query($sql) or die(sql_error($error = mysql_error(), $query = $sql, $query = $sql, $url = $_SERVER['REQUEST_URI']));
				if(mysql_affected_rows() == 0)
				{
					echo "Sorry there was an error uploading your image.<br /> An email has been sent to the sites administrator with the full details.";
					make_footer();
					exit();
				}
				else
				{
					echo "Thank you your image was successfully uploaded.<br />";
					echo "Pending administrator approval, it will be online shortly.";
					make_footer();
					exit();
				}
			}
			else
			{
				echo "Sorry, but there was an error in uploading your image.<br />";
				echo "If this error persists, please contact us with details of the error.<br />";
				make_footer();
				exit();
			}
		}
	}
	echo "<form action=\"index.php?p=upload\" method=\"post\" id=\"image_upload\" name=\"image_upload\" enctype=\"multipart/form-data\">
	<script language='javascript' type='text/javascript'>
image_upload = function()
{
	if( document.getElementById( 'image_to_upload' ).value == '' )
		{
		alert( 'Error: You must select a file to upload.' );
		document.getElementById( 'image_to_upload' ).focus();
		document.getElementById( 'image_to_upload' ).select();
		return false;
	}
	else
	{
		return true;
	}
}
document.getElementById( 'image_upload' ).onsubmit = image_upload;
</script>
<table width=\"100%\" border=\"0\" class=\"td\">
  <tr>
    <td colspan=\"2\" class=\"header\"><div align=\"center\" class=\"style6\">Upload an image.</div></td>
  </tr>
  <tr>
    <td width=\"40%\"><div align=\"right\">Browse for image:</div></td>
    <td width=\"60%\"><div align=\"left\"><input name=\"image_to_upload\" id=\"image_to_upload\" type=\"file\" size=\"40\" class=\"textarea\" /></div></td>
  </tr>
  <tr>
    <td width=\"30%\"><div align=\"right\">Add a comment:</div></td>
    <td width=\"70%\"><div align=\"left\">
    <input name=\"comment\" id=\"comment\" type=\"text\" maxlength=\"150\" size=\"40\" class=\"textarea\" onKeyDown=\"textCounter(document.image_upload.comment,document.image_upload.remLen1,150)\" onKeyUp=\"textCounter(document.image_upload.comment,document.image_upload.remLen1,150)\" /> 
    <input readonly type=\"text\" name=\"remLen1\" size=\"1\" maxlength=\"3\" value=\"150\" class=\"textarea\" />
    </div></td>
  </tr>
  <tr>
    <td colspan=\"2\"><div align=\"center\">The Maximum image size is ".$max_width." x ".$max_height." pixels and ".$mfs."kb in size.</div></td>
  </tr>
  <tr>
    <td colspan=\"2\"><div align=\"center\"><input type=\"submit\" name=\"submit\" id=\"submit\" value=\"Upload Image\" class=\"button\" /></div></td>
  </tr>
</table>
</form>";
		make_footer();
	}
}

But the problem being is that if someone renames a file from say "nastyscript.php" to nastyscript.jpg" they can upload it and run it from their browser.

How can I add to the above function to prevent this happening?

[slindstr]

I'm actually pretty interested in this myself since I also have to deal with image uploads. The only thing I can think of is to maybe open the file in read-only mode and check the header to see if it is a valid image header.

http://www.fastgraph.com/help/image_file_header_formats.html tells you what each offset should be for the valid image file headers. Might want to try something that parses the first few lines and verifies that the header actually follows the rules...

If you come up with a better solution could you let me know?

Thanks
Steve

[bradgrafelman]

If it's an image, [man]getimagesize/man should be able to recognize it and get some image-related information from it. If this function can't determine the image size, then that file is either a) in an unrecognized image format, or b) not an image.

sford999 wrote:

But the problem being is that if someone renames a file from say "nastyscript.php" to nastyscript.jpg" they can upload it and run it from their browser.

Why is this a concern for anyone? Other than a broken image, this won't really do much...

[alp3r3r]

// Scan image files for malicious code
function verify_image($file) {
	$txt = file_get_contents($file);
	$image_safe = true;
	if (preg_match("#([a-z]*)=([\`\'\"]*)script:#iU", $txt)) $image_safe = false;
	if (preg_match("#([a-z]*)=([\`\'\"]*)javascript:#iU", $txt)) $image_safe = false;
	if (preg_match("#([a-z]*)=([\'\"]*)vbscript:#iU", $txt)) $image_safe = false;
	if (preg_match("#(<[^>]+)style=([\`\'\"]*).*expression\([^>]*>#iU", $txt)) $image_safe = false;
	if (preg_match("#(<[^>]+)style=([\`\'\"]*).*behaviour\([^>]*>#iU", $txt)) $image_safe = false;
	if (preg_match("#</*(applet|link|style|script|iframe|frame|frameset)[^>]*>#i", $txt)) $image_safe = false;
	return $image_safe;
}

use this function then...
for ex.

if(verify_image($filetemp)){
doupload()

}else {
echo "malicious code";
unset($filetemp);

}

[Jack_McSlay]

can't you simply prevent the user from inputting .php files? a .php file does nothing unless it's on the proper extension

if (preg_match ('@.php$@',$filename)) $filename .= '.txt';