diff options
| -rw-r--r-- | system/libraries/Upload.php | 61 | ||||
| -rw-r--r-- | user_guide/changelog.html | 1 | ||||
| -rw-r--r-- | 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 + // <a, <body, <head, <html, <img, <plaintext, <pre, <script, <table, <title + // title is basically just in SVG, but we filter it anyhow + + if ( ! preg_match('/<(a|body|head|html|img|plaintext|pre|script|table|title)[\s>]/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: </p> <li>The <a href="libraries/unit_testing.html">Unit Test Class</a> now has an optional "notes" field available to it, and allows for discrete display of test result items using <kbd>$this->unit->set_test_items()</kbd>.</li> <li>Added a <kbd>$xss_clean</kbd> class variable to the XMLRPC library, enabling control over the use of the Security library's <kbd>xss_clean()</kbd> method.</li> <li>Added a <kbd>download()</kbd> method to the <a href="libraries/ftp.html">FTP library</a></li> + <li>Changed <kbd>do_xss_clean()</kbd> to return FALSE if the uploaded file fails XSS checks.</li> </ul> </li> <li>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);</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> |