Replace CI_Upload::clean_file_name() usage with CI_Security::sanitize_filename()

Also applied @xeptor's fix (a big thanks) to the sanitize_filename() method and added a changelog entry for it - fixes issue #73.

3 files changed, 13 insertions, 49 deletions

diff --git a/system/core/Security.php b/system/core/Security.php
index a6cd14a5f..7aae54efc 100644
--- a/system/core/Security.php
+++ b/system/core/Security.php
@@ -576,7 +576,15 @@ class CI_Security {
 		}
 
 		$str = remove_invisible_characters($str, FALSE);
-		return stripslashes(str_replace($bad, '', $str));
+
+		do
+		{
+			$old = $str;
+			$str = str_replace($bad, '', $str);
+		}
+		while ($old !== $str);
+
+		return stripslashes($str);
 	}
 
 	// ----------------------------------------------------------------
diff --git a/system/libraries/Upload.php b/system/libraries/Upload.php
index 1f0bd6a6e..814ea68a4 100644
--- a/system/libraries/Upload.php
+++ b/system/libraries/Upload.php
@@ -463,7 +463,8 @@ class CI_Upload {
 		}
 
 		// Sanitize the file name for security
-		$this->file_name = $this->clean_file_name($this->file_name);
+		$CI =& get_instance();
+		$this->file_name = $CI->security->sanitize_filename($this->file_name);
 
 		// Truncate the file name if it's too long
 		if ($this->max_filename > 0)
@@ -971,53 +972,6 @@ class CI_Upload {
 	// --------------------------------------------------------------------
 
 	/**
-	 * Clean the file name for security
-	 *
-	 * @param	string	$filename
-	 * @return	string
-	 */
-	public function clean_file_name($filename)
-	{
-		$bad = array(
-				'<!--', '-->',
-				"'", '"',
-				'<', '>',
-				'&', '$',
-				'=',
-				';',
-				'?',
-				'/',
-				'!',
-				'#',
-				'%20',
-				'%22',
-				'%3c',		// <
-				'%253c',	// <
-				'%3e',		// >
-				'%0e',		// >
-				'%28',		// (
-				'%29',		// )
-				'%2528',	// (
-				'%26',		// &
-				'%24',		// $
-				'%3f',		// ?
-				'%3b',		// ;
-				'%3d'		// =
-			);
-
-		do
-		{
-			$old_filename = $filename;
-			$filename = str_replace($bad, '', $filename);
-		}
-		while ($old_filename !== $filename);
-
-		return stripslashes($filename);
-	}
-
-	// --------------------------------------------------------------------
-
-	/**
 	 * Limit the File Name Length
 	 *
 	 * @param	string	$filename
diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index 982ae22f4..daa1cfc7a 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -215,6 +215,7 @@ Release Date: Not Released
 	 -  Added **max_filename_increment** config setting.
 	 -  Added an **index** parameter to the ``data()`` method.
 	 -  Added the **min_width** and **min_height** options for images.
+	 -  Removed method ``clean_file_name()`` and its usage in favor of :doc:`Security Library <libraries/security>`'s ``sanitize_filename()``.
    -  :doc:`Cart library <libraries/cart>` changes include:
 	 -  ``insert()`` now auto-increments quantity for an item when inserted twice instead of resetting it, this is the default behaviour of large e-commerce sites.
 	 -  *Product Name* strictness can be disabled by switching the ``$product_name_safe`` property to FALSE.
@@ -478,6 +479,7 @@ Bug fixes for 3.0
 -  Fixed a bug (#2061) - :doc:`Routing Class <general/routing>` didn't properly sanitize directory, controller and function triggers with **enable_query_strings** set to TRUE.
 -  Fixed a bug - SQLSRV didn't support ``escape_like_str()`` or escaping an array of values.
 -  Fixed a bug - :doc:`DB result <database/results>` method ``list_fields()`` didn't reset its field pointer for the *mysql*, *mysqli* and *mssql* drivers.
+-  Fixed a bug (#73) - :doc:`Security Library <libraries/security>` method ``sanitize_filename()`` could be tricked by an XSS attack.
 
 Version 2.1.3
 =============