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.
diff --git a/system/core/Security.php b/system/core/Security.php
index a6cd14a..7aae54e 100644
--- a/system/core/Security.php
+++ b/system/core/Security.php
@@ -576,7 +576,15 @@
}
$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 1f0bd6a..814ea68 100644
--- a/system/libraries/Upload.php
+++ b/system/libraries/Upload.php
@@ -463,7 +463,8 @@
}
// 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 @@
// --------------------------------------------------------------------
/**
- * 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 982ae22..daa1cfc 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -215,6 +215,7 @@
- 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 @@
- 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
=============