Upload Script Advice (emphasis on Security)

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
han866
Forum Newbie
Posts: 4
Joined: Sun Dec 04, 2005 8:25 pm

Upload Script Advice (emphasis on Security)

Post 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
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

Moved to PHP-Security.
han866
Forum Newbie
Posts: 4
Joined: Sun Dec 04, 2005 8:25 pm

Post 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!
Post Reply