diff options
| -rw-r--r-- | system/core/Router.php | 13 | ||||
| -rw-r--r-- | system/core/URI.php | 9 | ||||
| -rw-r--r-- | tests/codeigniter/core/URI_test.php | 20 | ||||
| -rw-r--r-- | user_guide_src/source/changelog.rst | 2 | ||||
| -rw-r--r-- | user_guide_src/source/installation/upgrade_300.rst | 28 |
5 files changed, 39 insertions, 33 deletions
diff --git a/system/core/Router.php b/system/core/Router.php index 7f18adbf5..d86735f5f 100644 --- a/system/core/Router.php +++ b/system/core/Router.php @@ -171,18 +171,21 @@ class CI_Router { $_d = isset($_GET[$_d]) ? trim($_GET[$_d], " \t\n\r\0\x0B/") : ''; if ($_d !== '') { - $this->set_directory($this->uri->filter_uri($_d)); + $this->uri->filter_uri($_d); + $this->set_directory($_d); } - $_c = $this->config->item('controller_trigger'); + $_c = trim($this->config->item('controller_trigger')); if ( ! empty($_GET[$_c])) { - $this->set_class(trim($this->uri->filter_uri(trim($_GET[$_c])))); + $this->uri->filter_uri($_GET[$_c]); + $this->set_class($_GET[$_c]); - $_f = $this->config->item('function_trigger'); + $_f = trim($this->config->item('function_trigger')); if ( ! empty($_GET[$_f])) { - $this->set_method(trim($this->uri->filter_uri($_GET[$_f]))); + $this->uri->filter_uri($_GET[$_f]); + $this->set_method($_GET[$_f]); } $this->uri->rsegments = array( diff --git a/system/core/URI.php b/system/core/URI.php index 067338d2a..790910169 100644 --- a/system/core/URI.php +++ b/system/core/URI.php @@ -173,8 +173,9 @@ class CI_URI { // Populate the segments array foreach (explode('/', trim($this->uri_string, '/')) as $val) { + $val = trim($val); // Filter segments for security - $val = trim($this->filter_uri($val)); + $this->filter_uri($val); if ($val !== '') { @@ -318,16 +319,14 @@ class CI_URI { * Filters segments for malicious characters. * * @param string $str - * @return string + * @return void */ - public function filter_uri($str) + public function filter_uri(&$str) { if ( ! empty($str) && ! empty($this->_permitted_uri_chars) && ! preg_match('/^['.$this->_permitted_uri_chars.']+$/i'.(UTF8_ENABLED ? 'u' : ''), $str)) { show_error('The URI you submitted has disallowed characters.', 400); } - - return $str; } // -------------------------------------------------------------------- diff --git a/tests/codeigniter/core/URI_test.php b/tests/codeigniter/core/URI_test.php index 4b1d644e3..adb62f6cb 100644 --- a/tests/codeigniter/core/URI_test.php +++ b/tests/codeigniter/core/URI_test.php @@ -119,26 +119,12 @@ class URI_test extends CI_TestCase { */ // -------------------------------------------------------------------- - public function test_filter_uri() + public function test_filter_uri_passing() { $this->uri->_set_permitted_uri_chars('a-z 0-9~%.:_\-'); - $str_in = 'abc01239~%.:_-'; - $str = $this->uri->filter_uri($str_in); - - $this->assertEquals($str, $str_in); - } - - // -------------------------------------------------------------------- - - public function test_filter_uri_escaping() - { - // ensure escaping even if dodgey characters are permitted - $this->uri->_set_permitted_uri_chars('a-z 0-9~%.:_\-()$'); - - $str = $this->uri->filter_uri('$destroy_app(foo)'); - - $this->assertEquals($str, '$destroy_app(foo)'); + $str = 'abc01239~%.:_-'; + $this->uri->filter_uri($str); } // -------------------------------------------------------------------- diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst index 38fd75971..ff319b8d2 100644 --- a/user_guide_src/source/changelog.rst +++ b/user_guide_src/source/changelog.rst @@ -57,6 +57,7 @@ Release Date: Not Released - Added support for changing the file extension of log files using ``$config['log_file_extension']``. - Added support for turning newline standardization on/off via ``$config['standardize_newlines']`` and set it to FALSE by default. - Added configuration setting ``$config['composer_autoload']`` to enable loading of a `Composer <https://getcomposer.org/>`_ auto-loader. + - Removed the automatic conversion of 'programmatic characters' to HTML entities from the :doc:`URI Library <libraries/uri>`. - Helpers @@ -445,6 +446,7 @@ Release Date: Not Released - Added conditional PCRE UTF-8 support to the "invalid URI characters" check and removed the ``preg_quote()`` call from it to allow more flexibility. - Renamed method ``_filter_uri()`` to ``filter_uri()``. + - Changed method ``filter_uri()`` to accept by reference and removed its return value. - Changed private methods to protected so that MY_URI can override them. - Renamed internal method ``_parse_cli_args()`` to ``_parse_argv()``. - Renamed internal method ``_detect_uri()`` to ``_parse_request_uri()``. diff --git a/user_guide_src/source/installation/upgrade_300.rst b/user_guide_src/source/installation/upgrade_300.rst index ef85106b7..2e9ee4e72 100644 --- a/user_guide_src/source/installation/upgrade_300.rst +++ b/user_guide_src/source/installation/upgrade_300.rst @@ -223,8 +223,24 @@ Otherwise however, please review your usage of the following functions: ``$_COOKIE`` and ``$_SERVER`` superglobals are no longer automatically overwritten when global XSS filtering is turned on. +************************************************* +Step 12: Check for potential XSS issues with URIs +************************************************* + +The :doc:`URI Library <../libraries/uri>` used to automatically convert +a certain set of "programmatic characters" to HTML entities when they +are encountered in a URI segment. + +This was aimed at providing some automatic XSS prodection, in addition +to the ``$config['permitted_uri_chars']`` setting, but has proven to be +problematic and is now removed in CodeIgniter 3.0. + +If your application has relied on this feature, you should update it to +filter URI segments through ``$this->security->xss_clean()`` whenever you +output them. + ******************************************************** -Step 12: Update usage of Input Class's get_post() method +Step 13: Update usage of Input Class's get_post() method ******************************************************** Previously, the :doc:`Input Class <../libraries/input>` method ``get_post()`` @@ -235,14 +251,14 @@ A method has been added, ``post_get()``, which searches in POST then in GET, as ``get_post()`` was doing before. *********************************************************************** -Step 13: Update usage of Directory Helper's directory_map() function +Step 14: Update usage of Directory Helper's directory_map() function *********************************************************************** In the resulting array, directories now end with a trailing directory separator (i.e. a slash, usually). ************************************************************* -Step 14: Update usage of Database Forge's drop_table() method +Step 15: Update usage of Database Forge's drop_table() method ************************************************************* Up until now, ``drop_table()`` added an IF EXISTS clause by default or it didn't work @@ -264,7 +280,7 @@ If your application relies on IF EXISTS, you'll have to change its usage. all drivers with the exception of ODBC. *********************************************************** -Step 15: Change usage of Email library with multiple emails +Step 16: Change usage of Email library with multiple emails *********************************************************** The :doc:`Email Library <../libraries/email>` will automatically clear the @@ -279,7 +295,7 @@ pass FALSE as the first parameter in the ``send()`` method: } *************************************************** -Step 16: Update your Form_validation language lines +Step 17: Update your Form_validation language lines *************************************************** Two improvements have been made to the :doc:`Form Validation Library @@ -310,7 +326,7 @@ files and error messages format: later. **************************************************************** -Step 17: Remove usage of (previously) deprecated functionalities +Step 18: Remove usage of (previously) deprecated functionalities **************************************************************** In addition to the ``$autoload['core']`` configuration setting, there's a |