From 009c8f09fbe767b01453f32b28f8a8a8dd4ef7c5 Mon Sep 17 00:00:00 2001 From: gommarah Date: Mon, 28 Jan 2013 13:45:50 +0200 Subject: Upload library, clean_file_name function: Fix xss bug. For example: If you clear this string "%%3f3f" according to the $bad array will fail. The result will be "%3f" Because str_replace() replaces left to right. Signed-off-by: xeptor --- system/libraries/Upload.php | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'system/libraries/Upload.php') diff --git a/system/libraries/Upload.php b/system/libraries/Upload.php index 96bb17edc..86c93411e 100644 --- a/system/libraries/Upload.php +++ b/system/libraries/Upload.php @@ -1005,6 +1005,13 @@ class CI_Upload { '%3d' // = ); + do + { + $old_filename = $filename; + $filename = str_replace($bad, '', $filename); + } + while ($old_filename !== $filename); + return stripslashes(str_replace($bad, '', $filename)); } -- cgit v1.2.3-24-g4f1b From 9be4cd74db158d805e0bc04c48c52a6453337c1d Mon Sep 17 00:00:00 2001 From: gommarah Date: Mon, 28 Jan 2013 13:58:35 +0200 Subject: Remove str_replace in return --- system/libraries/Upload.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system/libraries/Upload.php') diff --git a/system/libraries/Upload.php b/system/libraries/Upload.php index 86c93411e..1f0bd6a6e 100644 --- a/system/libraries/Upload.php +++ b/system/libraries/Upload.php @@ -1012,7 +1012,7 @@ class CI_Upload { } while ($old_filename !== $filename); - return stripslashes(str_replace($bad, '', $filename)); + return stripslashes($filename); } // -------------------------------------------------------------------- -- cgit v1.2.3-24-g4f1b From 7e5597782a589e4171ca08abdd9ce1a185542ff4 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Tue, 29 Jan 2013 15:38:33 +0200 Subject: 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. --- system/libraries/Upload.php | 50 ++------------------------------------------- 1 file changed, 2 insertions(+), 48 deletions(-) (limited to 'system/libraries/Upload.php') 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) @@ -970,53 +971,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 * -- cgit v1.2.3-24-g4f1b From 3567246091195e035ea4c8d3b2915eb6b45ad5e2 Mon Sep 17 00:00:00 2001 From: vlakoff Date: Fri, 15 Feb 2013 01:36:04 +0100 Subject: Various cosmetic fixes --- system/libraries/Upload.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'system/libraries/Upload.php') diff --git a/system/libraries/Upload.php b/system/libraries/Upload.php index 814ea68a4..acd76b03b 100644 --- a/system/libraries/Upload.php +++ b/system/libraries/Upload.php @@ -430,7 +430,7 @@ class CI_Upload { } else { - // An extension was provided, lets have it! + // An extension was provided, let's have it! $this->file_ext = $this->get_extension($this->_file_name_override); } @@ -1050,7 +1050,7 @@ class CI_Upload { // ]/i', $opening_bytes); } -- cgit v1.2.3-24-g4f1b From f399425bacfa461a77be2e841c3c1f48f9241c4a Mon Sep 17 00:00:00 2001 From: vlakoff Date: Tue, 19 Feb 2013 01:45:23 +0100 Subject: Fix a code comment in Upload->_file_mime_type() Availability of dangerous functions is now tested using function_usable(). --- system/libraries/Upload.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'system/libraries/Upload.php') diff --git a/system/libraries/Upload.php b/system/libraries/Upload.php index acd76b03b..1c14f99ed 100644 --- a/system/libraries/Upload.php +++ b/system/libraries/Upload.php @@ -1212,7 +1212,7 @@ class CI_Upload { * Notes: * - the DIRECTORY_SEPARATOR comparison ensures that we're not on a Windows system * - many system admins would disable the exec(), shell_exec(), popen() and similar functions - * due to security concerns, hence the function_exists() checks + * due to security concerns, hence the function_usable() checks */ if (DIRECTORY_SEPARATOR !== '\\') { @@ -1223,7 +1223,7 @@ class CI_Upload { if (function_usable('exec')) { /* This might look confusing, as $mime is being populated with all of the output when set in the second parameter. - * However, we only neeed the last line, which is the actual return value of exec(), and as such - it overwrites + * However, we only need the last line, which is the actual return value of exec(), and as such - it overwrites * anything that could already be set for $mime previously. This effectively makes the second parameter a dummy * value, which is only put to allow us to get the return status code. */ -- cgit v1.2.3-24-g4f1b