summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrey Andreev <narf@devilix.net>2014-12-05 11:00:11 +0100
committerAndrey Andreev <narf@devilix.net>2014-12-05 11:00:11 +0100
commitbfa233f559a50ee0674a209fa56f866edc814fd9 (patch)
treedfdf0aa87b31db69c7c6646b8bbfbc743797edcd
parentbc11439c5f7fbbb1ef0257f8083c375eeb9dd79c (diff)
Further changes related to issue #47, PR #3323
- Removed a test that was created specifically for the 'convert programmatic characters to entities' feature. - Changed filter_uri() to accept by reference and to not return anything as its only purpose now is to trigger a show_error() call. - Added changelog messages and updated the upgrade instructions.
-rw-r--r--system/core/Router.php13
-rw-r--r--system/core/URI.php9
-rw-r--r--tests/codeigniter/core/URI_test.php20
-rw-r--r--user_guide_src/source/changelog.rst2
-rw-r--r--user_guide_src/source/installation/upgrade_300.rst28
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