Page 1 of 1

Upload Script Advice (emphasis on Security)

Posted: Sun Dec 04, 2005 8:36 pm
by han866
Hello

I'm trying to tighten up my upload script(s). Can one of you gurus check it out.. and let me know if I'm forgetting anything, or if something is blatantly bad.

The emphasis is on the checkType() function. Basically, I want to have the tmp file go through a series of checks before moving the file.

1. Check the $_FILES[fieldname][type].
I know that this is trivial to bypass

2. Check the MIME type using the UNIX 'file'

3. If it is an image, do another check using exif_imagetype().

Errors will be emailed.

Additional security features exist on the server..
1. mod_Security (using SecUploadApproveScript)
2. uploads are uploaded above the web root
3. /tmp partition is mounted noexec, nosuid
4. safe_mode is enabled (safe_mode_exec_dir points to a directory which contains 'file' only)
5. openbase_dir restrictions are in place

Code: Select all

define("ERROR_EMAIL",		"admin@mydomain.com");
function reportMail($inSubject, $inMessage){
	$headers 	= "From:error@mydomain.com".chr(10);
	$headers 	.= "Content-Type: text/plain;\n\tcharset=\"iso-8859-1\"".chr(10);
	mail(ERROR_EMAIL, $inSubject, $inMessage, $headers);
}


function checkType($inMimeArray, $fieldName){
	$FILES_type_pass		= false;
	$MIME_type_pass			= false;
	$IMAGE_type_check		= false;
	$imgArray				= array(
								"images/gif",
								"images/jpeg",
								"images/pjpeg",
								"images/tiff",
								"images/x-xbitmap",
								"images/x-xpixmap",
								"images/x-portable-bitmap",
								"images/x-portable-graymap",
								"images/png");
	$imgTypeConstants		= array(
								"IMAGETYPE_GIF",
								"IMAGETYPE_JPEG",
								"IMAGETYPE_PNG",
								"IMAGETYPE_BMP",
								"IMAGETYPE_TIFF_II",
								"IMAGETYPE_TIFF_MM");
	$testFile				= escapeshellcmd($_FILES[$fieldName]['tmp_name']);
	$xCMD					= "file -ib ".$testFile;
	$tmpMIME				= trim(exec($xCMD, $errOut, $retOut));
	if($retOut){
		reportMail("Binary [file] check error", "Command: ".$xCMD.chr(10)."Error Array: ".serialize($errOut).chr(10)."return: ".$retOut);
		exit();
	}
	$tmpArr					= explode(";", $tmpMIME);
	if(sizeof($tmpArr)){
		$tmpMIME		 	= trim($tmpArr[0]);
	}
	$checkImage				= (in_array($tmpMIME, $imgArray) ? true : false);
	if($_FILES[$fieldName]["error"] == "0"){
		if(is_array($inMimeArray) && sizeof($inMimeArray)){
			$FILES_type_pass  	= (in_array($_FILES[$fieldName]["type"], $inMimeArray) ? true : false);
			$MIME_type_pass		= (in_array($tmpMIME, $inMimeArray) ? true : false);
			if($checkImage){
				foreach($imgTypeConstants as $constantVal){
					if(exif_imagetype($_FILES[$fieldName]['tmp_name'], $constantVal)){
						$IMAGE_type_check	= true;
						break;
					}
				}
				$MIME_type_pass	= ($IMAGE_type_check ? true : false);
			}
		}else{
			unlink($_FILES[$fieldName]["tmp_name"]);
			reportMail("invalid MIME array error", serialize($_FILES).chr(10).chr(10).serialize($inMimeArray));
			exit();
		}
	}else{
		reportMail("_FILES error", serialize($_FILES));
		exit();
	}
	$retVal		= ($FILES_type_pass && $MIME_type_pass ? true : false);
	if(!$retVal){
		$msg				= "\$_FILES[] = ".serialize($_FILES).chr(10).chr(10);
		$msg				.= "Allowed types = ".serialize($inMimeArray).chr(10).chr(10);
		$msg				.= "Actual MIME type = ".$tmpMIME;
		reportMail("_FILES checkTypeFail", $msg);
	}
	return $retVal;
}


if(isset($_FILES["UploadFile"]["size"]) && $_FILES["UploadFile"]["size"] > 0){
	if(is_uploaded_file($_FILES['UploadFile']['tmp_name'])){
		$allowedTypes			= array("image/gif",
										"image/jpeg",
										"image/pjpeg");
		if(checkType($allowedTypes,"UploadFile")){
			$filePath			= "/var/www/localhost/htdocs/misc/test/".$_FILES['UploadFile']['name'];
			if(!move_uploaded_file($_FILES['UploadFile']['tmp_name'], $filePath)){
				echo "Unable to move";
			}else{
				chmod($filePath,0644);
			}
		}
	}else{
		reportMail("is_uploaded_files FAILED", serialize($_FILES));
	}
}
?>
<form action="upload.php" method="post" name="UploadForm" enctype="multipart/form-data">
<strong>FILE:</strong> <input type="file" name="UploadFile"><br>
<input type="submit" value="Upload">
</form>

Please let me know your thoughts. Anything to improve security is greatly encouraged.
Thanks !!!

han866

Posted: Sun Dec 04, 2005 8:41 pm
by John Cartwright
Moved to PHP-Security.

Posted: Mon Dec 05, 2005 11:37 pm
by han866
I made an adjustment.. I wasn't liking the exec() call using file. I was having a hell of a time getting mime_content_type() working.. but discovered the PECL fileinfo wrapper -> libmagic. Gave it a whirl.. and I think I like it. Anyone have any experience working with this? Are there security issues related with this wrapper?

I've added my new code below:

Code: Select all

<?
define("ERROR_EMAIL",		"support@mydomain.com");
function reportMail($inSubject, $inMessage){
	$headers 	= "From:error@mydomain.com".chr(10);
	$headers 	.= "Content-Type: text/plain;\n\tcharset=\"iso-8859-1\"".chr(10);
	mail(ERROR_EMAIL, $inSubject, $inMessage, $headers);
}


function checkType($inMimeArray, $fieldName){
	$FILES_type_pass		= false;
	$MIME_type_pass			= false;
	$IMAGE_type_check		= false;
	$imgArray				= array(
								"images/gif",
								"images/jpeg",
								"images/pjpeg",
								"images/tiff",
								"images/x-xbitmap",
								"images/x-xpixmap",
								"images/x-portable-bitmap",
								"images/x-portable-graymap",
								"images/png");
	$imgTypeConstants		= array(
								"IMAGETYPE_GIF",
								"IMAGETYPE_JPEG",
								"IMAGETYPE_PNG",
								"IMAGETYPE_BMP",
								"IMAGETYPE_TIFF_II",
								"IMAGETYPE_TIFF_MM");
	$testFile				= escapeshellcmd($_FILES[$fieldName]['tmp_name']);
	// PECL fileinfo 
	// http://pecl.php.net/package/fileinfo
	// http://wiki.cc/php/Fileinfo
	// http://us2.php.net/mime_magic
	$res					= finfo_open(FILEINFO_MIME);
	$tmpMIME				= finfo_file($res,$testFile);
	finfo_close($res);
	// end PECL fileinfo
	$checkImage				= (in_array($tmpMIME, $imgArray) ? true : false);
	if($_FILES[$fieldName]["error"] == "0"){
		if(is_array($inMimeArray) && sizeof($inMimeArray)){
			$FILES_type_pass  	= (in_array($_FILES[$fieldName]["type"], $inMimeArray) ? true : false);
			$MIME_type_pass		= (in_array($tmpMIME, $inMimeArray) ? true : false);
			if($checkImage){
				foreach($imgTypeConstants as $constantVal){
					if(exif_imagetype($_FILES[$fieldName]['tmp_name'], $constantVal)){
						$IMAGE_type_check	= true;
						break;
					}
				}
				$MIME_type_pass	= ($IMAGE_type_check ? true : false);
			}
		}else{
			unlink($_FILES[$fieldName]["tmp_name"]);
			reportMail("invalid MIME array error", serialize($_FILES).chr(10).chr(10).serialize($inMimeArray));
			exit();
		}
	}else{
		reportMail("_FILES error", serialize($_FILES));
		exit();
	}
	$retVal		= ($FILES_type_pass && $MIME_type_pass ? true : false);
	if(!$retVal){
		$msg				= "\$_FILES[] = ".serialize($_FILES).chr(10).chr(10);
		$msg				.= "Allowed types = ".serialize($inMimeArray).chr(10).chr(10);
		$msg				.= "Actual MIME type = ".$tmpMIME;
		reportMail("_FILES checkTypeFail", $msg);
	}
	return $retVal;
}


if(isset($_FILES["UploadFile"]["size"]) && $_FILES["UploadFile"]["size"] > 0){
	if(is_uploaded_file($_FILES['UploadFile']['tmp_name'])){
		$allowedTypes			= array("image/gif",
										"image/jpeg",
										"image/pjpeg");
		if(checkType($allowedTypes,"UploadFile")){
			$filePath			= "/var/www/localhost/htdocs/misc/test/".$_FILES['UploadFile']['name'];
			if(!move_uploaded_file($_FILES['UploadFile']['tmp_name'], $filePath)){
				echo "Unable to move";
			}else{
				chmod($filePath,0644);
			}
		}
	}else{
		reportMail("is_uploaded_files FAILED", serialize($_FILES));
	}
}
?>
<form action="upload.php" method="post" name="UploadForm" enctype="multipart/form-data">
<strong>FILE:</strong> <input type="file" name="UploadFile"><br>
<input type="submit" value="Upload">
</form>
Thanks!