From f82e51cd8f46b112c3c400d43db9044854a8e805 Mon Sep 17 00:00:00 2001
From: Greg Aker
Date: Wed, 14 Apr 2010 19:33:50 -0500
Subject: Update to File Upload library to return boolean on do_xss_clean().
---
system/libraries/Upload.php | 61 +++++++++++++++++++++++++-------
user_guide/changelog.html | 1 +
user_guide/libraries/file_uploading.html | 6 ++++
3 files changed, 55 insertions(+), 13 deletions(-)
diff --git a/system/libraries/Upload.php b/system/libraries/Upload.php
index ac9323c08..3131469de 100644
--- a/system/libraries/Upload.php
+++ b/system/libraries/Upload.php
@@ -276,9 +276,13 @@ class CI_Upload {
* embedded within a file. Scripts can easily
* be disguised as images or other file types.
*/
- if ($this->xss_clean == TRUE)
+ if ($this->xss_clean)
{
- $this->do_xss_clean();
+ if ($this->do_xss_clean() === FALSE)
+ {
+ $this->set_error('upload_unable_to_write_file');
+ return FALSE;
+ }
}
/*
@@ -803,24 +807,55 @@ class CI_Upload {
{
return FALSE;
}
+
+ if (function_exists('memory_get_usage') && memory_get_usage() && ini_get('memory_limit') != '')
+ {
+ $current = ini_get('memory_limit') * 1024 * 1024;
+
+ // There was a bug/behavioural change in PHP 5.2, where numbers over one million get output
+ // into scientific notation. number_format() ensures this number is an integer
+ // http://bugs.php.net/bug.php?id=43053
+
+ $new_memory = number_format(ceil(filesize($this->new_name) + $current), 0, '.', '');
+
+ ini_set('memory_limit', $new_memory); // When an integer is used, the value is measured in bytes. - PHP.net
+ }
- if (($data = @file_get_contents($file)) === FALSE)
+ // If the file being uploaded is an image, then we should have no problem with XSS attacks (in theory), but
+ // IE can be fooled into mime-type detecting a malformed image as an html file, thus executing an XSS attack on anyone
+ // using IE who looks at the image. It does this by inspecting the first 255 bytes of an image. To get around this
+ // CI will itself look at the first 255 bytes of an image to determine its relative safety. This can save a lot of
+ // processor power and time if it is actually a clean image, as it will be in nearly all instances _except_ an
+ // attempted XSS attack.
+
+ if (function_exists('getimagesize') && @getimagesize($file) !== FALSE)
{
- return FALSE;
+ if (($file = @fopen($file, 'rb')) === FALSE) // "b" to force binary
+ {
+ return FALSE; // Couldn't open the file, return FALSE
+ }
+
+ $opening_bytes = fread($file, 256);
+ fclose($file);
+
+ // These are known to throw IE into mime-type detection chaos
+ // ]/i', $opening_bytes))
+ {
+ return TRUE; // its an image, no "triggers" detected in the first 256 bytes, we're good
+ }
}
-
- if ( ! $fp = @fopen($file, FOPEN_READ_WRITE))
+
+ if (($data = @file_get_contents($file)) === FALSE)
{
return FALSE;
}
- $CI =& get_instance();
- $data = $CI->security->xss_clean($data);
-
- flock($fp, LOCK_EX);
- fwrite($fp, $data);
- flock($fp, LOCK_UN);
- fclose($fp);
+ $CI =& get_instance();
+
+ return $CI->security->xss_clean($data, TRUE);
}
// --------------------------------------------------------------------
diff --git a/user_guide/changelog.html b/user_guide/changelog.html
index 5e0f5ae05..25b3b1744 100644
--- a/user_guide/changelog.html
+++ b/user_guide/changelog.html
@@ -93,6 +93,7 @@ Hg Tag:
The Unit Test Class now has an optional "notes" field available to it, and allows for discrete display of test result items using $this->unit->set_test_items().
Added a $xss_clean class variable to the XMLRPC library, enabling control over the use of the Security library's xss_clean() method.
Added a download() method to the FTP library
+ Changed do_xss_clean() to return FALSE if the uploaded file fails XSS checks.
Database
diff --git a/user_guide/libraries/file_uploading.html b/user_guide/libraries/file_uploading.html
index d143f5b6e..061d55627 100644
--- a/user_guide/libraries/file_uploading.html
+++ b/user_guide/libraries/file_uploading.html
@@ -318,6 +318,12 @@ $this->upload->initialize($config);
TRUE/FALSE (boolean) |
If set to TRUE, any spaces in the file name will be converted to underscores. This is recommended. |
+
+xss_clean |
+FALSE |
+TRUE/FALSE (boolean) |
+If set to TRUE, the files will be tested for XSS vulnerabilities. |
+
--
cgit v1.2.3-24-g4f1b