201007 file upload bug fix

2 files changed, 118 insertions, 97 deletions

diff --git a/system/libraries/Upload.php b/system/libraries/Upload.php
index 8bdb4be19..c94e4d448 100644
--- a/system/libraries/Upload.php
+++ b/system/libraries/Upload.php
@@ -26,31 +26,34 @@
  */
 class CI_Upload {
 	
-	var $max_size		= 0;
-	var $max_width		= 0;
-	var $max_height		= 0;
-	var $max_filename	= 0;
-	var $allowed_types	= "";
-	var $file_temp		= "";
-	var $file_name		= "";
-	var $orig_name		= "";
-	var $file_type		= "";
-	var $file_size		= "";
-	var $file_ext		= "";
-	var $upload_path	= "";
-	var $overwrite		= FALSE;
-	var $encrypt_name	= FALSE;
-	var $is_image		= FALSE;
-	var $image_width	= '';
-	var $image_height	= '';
-	var $image_type		= '';
-	var $image_size_str	= '';
-	var $error_msg		= array();
-	var $mimes			= array();
-	var $remove_spaces	= TRUE;
-	var $xss_clean		= FALSE;
-	var $temp_prefix	= "temp_file_";
-		
+	var $max_size				= 0;
+	var $max_width				= 0;
+	var $max_height				= 0;
+	var $max_filename			= 0;
+	var $allowed_types			= "";
+	var $file_temp				= "";
+	var $file_name				= "";
+	var $orig_name				= "";
+	var $file_type				= "";
+	var $file_size				= "";
+	var $file_ext				= "";
+	var $upload_path			= "";
+	var $overwrite				= FALSE;
+	var $encrypt_name			= FALSE;
+	var $is_image				= FALSE;
+	var $image_width			= '';
+	var $image_height			= '';
+	var $image_type				= '';
+	var $image_size_str			= '';
+	var $error_msg				= array();
+	var $mimes					= array();
+	var $remove_spaces			= TRUE;
+	var $xss_clean				= FALSE;
+	var $temp_prefix			= "temp_file_";
+	var $client_name			= '';
+	
+	var $_file_name_override	= ''; //@PHP4 (should be private)
+	
 	/**
 	 * Constructor
 	 *
@@ -75,6 +78,7 @@ class CI_Upload {
 	 * @param	array
 	 * @return	void
 	 */	
+
 	function initialize($config = array())
 	{
 		$defaults = array(
@@ -101,7 +105,8 @@ class CI_Upload {
 							'mimes'				=> array(),
 							'remove_spaces'		=> TRUE,
 							'xss_clean'			=> FALSE,
-							'temp_prefix'		=> "temp_file_"
+							'temp_prefix'		=> "temp_file_",
+							'client_name'		=> ''
 						);	
 	
 	
@@ -124,6 +129,10 @@ class CI_Upload {
 				$this->$key = $val;
 			}
 		}
+		
+		// if a file_name was provided in the config, use it instead of the user input
+		// supplied file name for all uploads until initialized again
+		$this->_file_name_override = $this->file_name;
 	}
 	
 	// --------------------------------------------------------------------
@@ -185,20 +194,16 @@ class CI_Upload {
 			return FALSE;
 		}
 
+
 		// Set the uploaded data as class variables
 		$this->file_temp = $_FILES[$field]['tmp_name'];		
-		$this->file_name = $this->_prep_filename($_FILES[$field]['name']);
-		$this->file_size = $_FILES[$field]['size'];		
+		$this->file_size = $_FILES[$field]['size'];	
 		$this->file_type = preg_replace("/^(.+?);.*$/", "\\1", $_FILES[$field]['type']);
 		$this->file_type = strtolower(trim(stripslashes($this->file_type), '"'));
-		$this->file_ext	 = $this->get_extension($_FILES[$field]['name']);
+		$this->file_name = $this->_prep_filename($_FILES[$field]['name']);
+		$this->file_ext	 = $this->get_extension($this->file_name);
+		$this->client_name = $this->file_name;
 		
-		// Convert the file size to kilobytes
-		if ($this->file_size > 0)
-		{
-			$this->file_size = round($this->file_size/1024, 2);
-		}
-
 		// Is the file type allowed to be uploaded?
 		if ( ! $this->is_allowed_filetype())
 		{
@@ -206,6 +211,25 @@ class CI_Upload {
 			return FALSE;
 		}
 
+		// if we're overriding, let's now make sure the new name and type is allowed
+		if ($this->_file_name_override != '')
+		{
+			$this->file_name = $this->_prep_filename($this->_file_name_override);
+			$this->file_ext  = $this->get_extension($this->file_name);
+
+			if ( ! $this->is_allowed_filetype(TRUE))
+			{
+				$this->set_error('upload_invalid_filetype');
+				return FALSE;				
+			}
+		}
+		
+		// Convert the file size to kilobytes
+		if ($this->file_size > 0)
+		{
+			$this->file_size = round($this->file_size/1024, 2);
+		}
+
 		// Is the file size within the allowed maximum?
 		if ( ! $this->is_allowed_filesize())
 		{
@@ -255,6 +279,21 @@ class CI_Upload {
 		}
 
 		/*
+		 * Run the file through the XSS hacking filter
+		 * This helps prevent malicious code from being
+		 * embedded within a file.  Scripts can easily
+		 * be disguised as images or other file types.
+		 */
+		if ($this->xss_clean)
+		{
+			if ($this->do_xss_clean() === FALSE)
+			{
+				$this->set_error('upload_unable_to_write_file');
+				return FALSE;
+			}
+		}
+
+		/*
 		 * Move the file to the final destination
 		 * To deal with different server configurations
 		 * we'll attempt to use copy() first.  If that fails
@@ -269,21 +308,6 @@ class CI_Upload {
 				 return FALSE;
 			}
 		}
-		
-		/*
-		 * Run the file through the XSS hacking filter
-		 * This helps prevent malicious code from being
-		 * embedded within a file.  Scripts can easily
-		 * be disguised as images or other file types.
-		 */
-		if ($this->xss_clean)
-		{
-			if ($this->do_xss_clean() === FALSE)
-			{
-				$this->set_error('upload_unable_to_write_file');
-				return FALSE;
-			}
-		}
 
 		/*
 		 * Set the finalized image dimensions
@@ -316,6 +340,7 @@ class CI_Upload {
 						'full_path'			=> $this->upload_path.$this->file_name,
 						'raw_name'			=> str_replace($this->file_ext, '', $this->file_name),
 						'orig_name'			=> $this->orig_name,
+						'client_name'		=> $this->client_name,
 						'file_ext'			=> $this->file_ext,
 						'file_size'			=> $this->file_size,
 						'is_image'			=> $this->is_image(),
@@ -558,7 +583,7 @@ class CI_Upload {
 	 * @access	public
 	 * @return	bool
 	 */	
-	function is_allowed_filetype()
+	function is_allowed_filetype($ignore_mime = FALSE)
 	{
 		if ($this->allowed_types == '*')
 		{
@@ -570,36 +595,42 @@ class CI_Upload {
 			$this->set_error('upload_no_file_types');
 			return FALSE;
 		}
+		
+		$ext = strtolower(ltrim($this->file_ext, '.'));
+		
+		if ( ! in_array($ext, $this->allowed_types))
+		{
+			return FALSE;
+		}
 
+		// Images get some additional checks		
 		$image_types = array('gif', 'jpg', 'jpeg', 'png', 'jpe');
-
-		foreach ($this->allowed_types as $val)
+		
+		if (in_array($ext, $image_types))
 		{
-			$mime = $this->mimes_types(strtolower($val));
-
-			// Images get some additional checks
-			if ($this->file_ext == '.'.$val &&  in_array($val, $image_types))
+			if (getimagesize($this->file_temp) === FALSE)
 			{
-				if (getimagesize($this->file_temp) === FALSE)
-				{
-					return FALSE;
-				}
-			}
-
-			if (is_array($mime))
-			{
-				if (in_array($this->file_type, $mime, TRUE))
-				{
-					return TRUE;
-				}
-			}
-			else
+				return FALSE;
+			}			
+		}
+		
+		if ($ignore_mime === TRUE)
+		{
+			return TRUE;
+		}
+		
+		$mime = $this->mimes_types($ext);
+				
+		if (is_array($mime))
+		{
+			if (in_array($this->file_type, $mime, TRUE))
 			{
-				if ($mime == $this->file_type)
-				{
-					return TRUE;
-				}	
-			}		
+				return TRUE;
+			}			
+		}
+		elseif ($mime == $this->file_type)
+		{
+				return TRUE;
 		}
 		
 		return FALSE;
@@ -801,7 +832,7 @@ class CI_Upload {
 	 */	
 	function do_xss_clean()
 	{		
-		$file = $this->upload_path.$this->file_name;
+		$file = $this->file_temp;
 		
 		if (filesize($file) == 0)
 		{
@@ -968,7 +999,7 @@ class CI_Upload {
 
 		foreach ($parts as $part)
 		{
-			if ($this->mimes_types(strtolower($part)) === FALSE)
+			if ( ! in_array(strtolower($part), $this->allowed_types) OR $this->mimes_types(strtolower($part)) === FALSE)
 			{
 				$filename .= '.'.$part.'_';
 			}
@@ -978,13 +1009,6 @@ class CI_Upload {
 			}
 		}
 
-		// file name override, since the exact name is provided, no need to
-		// run it through a $this->mimes check.
-		if ($this->file_name != '')
-		{
-			$filename = $this->file_name;
-		}
-
 		$filename .= '.'.$ext;
 		
 		return $filename;
diff --git a/user_guide/libraries/file_uploading.html b/user_guide/libraries/file_uploading.html
index 061d55627..28cf8de3e 100644
--- a/user_guide/libraries/file_uploading.html
+++ b/user_guide/libraries/file_uploading.html
@@ -28,7 +28,7 @@
 <div id="masthead">
 <table cellpadding="0" cellspacing="0" border="0" style="width:100%">
 <tr>
-<td><h1>CodeIgniter User Guide Version 2.0.0</h1></td>
+<td><h1>CodeIgniter User Guide Version 1.7.2</h1></td>
 <td id="breadcrumb_right"><a href="../toc.html">Table of Contents Page</a></td>
 </tr>
 </table>
@@ -74,7 +74,7 @@ preferences, restricting the type and size of the files.</p>
 <li>Once uploaded, the user will be shown a success message.</li>
 </ul>
 
-<p>To demonstrate this process here is a brief tutorial. Afterward you'll find reference information.</p>
+<p>To demonstrate this process here is brief tutorial. Afterward you'll find reference information.</p>
 
 <h2>Creating the Upload Form</h2>
 
@@ -255,7 +255,7 @@ $this->upload->initialize($config);</code>
 <td class="td"><strong>allowed_types</strong></td>
 <td class="td">None</td>
 <td class="td">None</td>
-<td class="td">The mime types corresponding to the types of files you allow to be uploaded.  Usually the file extension can be used as the mime type.  Separate multiple types with a pipe.  If '*' is used, all file types will be allowed.</td>
+<td class="td">The mime types corresponding to the types of files you allow to be uploaded.  Usually the file extension can be used as the mime type.  Separate multiple types with a pipe.</td>
 </tr>
 
 
@@ -264,8 +264,7 @@ $this->upload->initialize($config);</code>
 <td class="td">None</td>
 <td class="td">Desired file name</td>
 <td class="td">
-	<p>If set CodeIgniter will rename the uploaded file to this name.</p>
-	<p class="important"><strong>Note:</strong>The filename should not include a file extension.</p>
+	<p>If set CodeIgniter will rename the uploaded file to this name.  The extension provided in the file name must also be an allowed file type.</p>
 </td>
 </tr>
 
@@ -318,12 +317,6 @@ $this->upload->initialize($config);</code>
 <td class="td">TRUE/FALSE (boolean)</td>
 <td class="td">If set to TRUE, any spaces in the file name will be converted to underscores. This is recommended.</td>
 </tr>
-<tr>
-<td class="td"><strong>xss_clean</strong></td>
-<td class="td">FALSE</td>
-<td class="td">TRUE/FALSE (boolean)</td>
-<td class="td">If set to TRUE, the files will be tested for XSS vulnerabilities.</td> 
-</tr>
 </table>
 
 
@@ -377,6 +370,7 @@ Here is the array prototype:</p>
 &nbsp;&nbsp;&nbsp;&nbsp;[full_path]&nbsp;&nbsp;&nbsp;&nbsp;=> /path/to/your/upload/jpg.jpg<br />
 &nbsp;&nbsp;&nbsp;&nbsp;[raw_name]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;=> mypic<br />
 &nbsp;&nbsp;&nbsp;&nbsp;[orig_name]&nbsp;&nbsp;&nbsp;&nbsp;=> mypic.jpg<br />
+&nbsp;&nbsp;&nbsp;&nbsp;[client_name]&nbsp;&nbsp;=> mypic.jpg<br />
 &nbsp;&nbsp;&nbsp;&nbsp;[file_ext]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;=> .jpg<br />
 &nbsp;&nbsp;&nbsp;&nbsp;[file_size]&nbsp;&nbsp;&nbsp;&nbsp;=> 22.2<br />
 &nbsp;&nbsp;&nbsp;&nbsp;[is_image]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;=> 1<br />
@@ -411,6 +405,9 @@ Here is the array prototype:</p>
 <tr><td class="td"><strong>orig_name</strong></td>
 <td class="td">The original file name.  This is only useful if you use the encrypted name option.</td></tr>
 
+<tr><td class="td"><strong>client_name</strong></td>
+<td class="td">The file name as supplied by the client user agent, prior to any file name preparation or incrementing.</td></tr>
+
 <tr><td class="td"><strong>file_ext</strong></td>
 <td class="td">The file extension with period</td></tr>
 
@@ -447,7 +444,7 @@ Previous Topic:&nbsp;&nbsp;<a href="encryption.html">Encryption Helper</a>
 <a href="../index.html">User Guide Home</a>&nbsp;&nbsp;&nbsp;&middot;&nbsp;&nbsp;
 Next Topic:&nbsp;&nbsp;<a href="form_validation.html">Form Validation Class</a>
 </p>
-<p><a href="http://codeigniter.com">CodeIgniter</a> &nbsp;&middot;&nbsp; Copyright &#169; 2006-2010 &nbsp;&middot;&nbsp; <a href="http://ellislab.com/">EllisLab, Inc.</a></p>
+<p><a href="http://codeigniter.com">CodeIgniter</a> &nbsp;&middot;&nbsp; Copyright &#169; 2006-2010 &nbsp;&middot;&nbsp; <a href="http://ellislab.com/">Ellislab, Inc.</a></p>
 </div>
 
 </body>