From ff773c059cb984920767dd6187c30a77e5bf78c9 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Wed, 14 Dec 2016 13:45:45 +0200 Subject: Finally drop CI_Input::_sanitize_globals() Close #4101 --- system/core/Input.php | 168 +-------------------- tests/mocks/core/input.php | 19 +-- user_guide_src/source/installation/upgrade_320.rst | 18 +++ user_guide_src/source/libraries/input.rst | 28 +--- 4 files changed, 32 insertions(+), 201 deletions(-) diff --git a/system/core/Input.php b/system/core/Input.php index d4f79ee68..aefc3b7d8 100644 --- a/system/core/Input.php +++ b/system/core/Input.php @@ -93,8 +93,15 @@ class CI_Input { */ protected $_input_stream; + /** + * CI_Security instance + * + * Used for the optional $xss_filter parameter that most + * getter methods have here. + * + * @var CI_Security + */ protected $security; - protected $uni; // -------------------------------------------------------------------- @@ -112,15 +119,6 @@ class CI_Input { $this->security =& load_class('Security', 'core'); - // Do we need the UTF-8 class? - if (UTF8_ENABLED === TRUE) - { - $this->uni =& load_class('Utf8', 'core'); - } - - // Sanitize global arrays - $this->_sanitize_globals(); - // CSRF Protection check if ($this->_enable_csrf === TRUE && ! is_cli()) { @@ -554,156 +552,6 @@ class CI_Input { // -------------------------------------------------------------------- - /** - * Sanitize Globals - * - * Internal method serving for the following purposes: - * - * - Unsets $_GET data, if query strings are not enabled - * - Cleans POST, COOKIE and SERVER data - * - * @return void - */ - protected function _sanitize_globals() - { - // Is $_GET data allowed? If not we'll set the $_GET to an empty array - if (is_array($_GET)) - { - foreach ($_GET as $key => $val) - { - $_GET[$this->_clean_input_keys($key)] = $this->_clean_input_data($val); - } - } - - // Clean $_POST Data - if (is_array($_POST)) - { - foreach ($_POST as $key => $val) - { - $_POST[$this->_clean_input_keys($key)] = $this->_clean_input_data($val); - } - } - - // Clean $_COOKIE Data - if (is_array($_COOKIE)) - { - // Also get rid of specially treated cookies that might be set by a server - // or silly application, that are of no use to a CI application anyway - // but that when present will trip our 'Disallowed Key Characters' alarm - // http://www.ietf.org/rfc/rfc2109.txt - // note that the key names below are single quoted strings, and are not PHP variables - unset( - $_COOKIE['$Version'], - $_COOKIE['$Path'], - $_COOKIE['$Domain'] - ); - - foreach ($_COOKIE as $key => $val) - { - if (($cookie_key = $this->_clean_input_keys($key)) !== FALSE) - { - $_COOKIE[$cookie_key] = $this->_clean_input_data($val); - } - else - { - unset($_COOKIE[$key]); - } - } - } - - // Sanitize PHP_SELF - $_SERVER['PHP_SELF'] = strip_tags($_SERVER['PHP_SELF']); - - log_message('info', 'Global POST, GET and COOKIE data sanitized'); - } - - // -------------------------------------------------------------------- - - /** - * Clean Input Data - * - * Internal method that aids in escaping data and - * standardizing newline characters to PHP_EOL. - * - * @param string|string[] $str Input string(s) - * @return string - */ - protected function _clean_input_data($str) - { - if (is_array($str)) - { - $new_array = array(); - foreach (array_keys($str) as $key) - { - $new_array[$this->_clean_input_keys($key)] = $this->_clean_input_data($str[$key]); - } - return $new_array; - } - - /* We strip slashes if magic quotes is on to keep things consistent - - NOTE: In PHP 5.4 get_magic_quotes_gpc() will always return 0 and - it will probably not exist in future versions at all. - */ - if ( ! is_php('5.4') && get_magic_quotes_gpc()) - { - $str = stripslashes($str); - } - - // Clean UTF-8 if supported - if (UTF8_ENABLED === TRUE) - { - $str = $this->uni->clean_string($str); - } - - // Remove control characters - $str = remove_invisible_characters($str, FALSE); - - return $str; - } - - // -------------------------------------------------------------------- - - /** - * Clean Keys - * - * Internal method that helps to prevent malicious users - * from trying to exploit keys we make sure that keys are - * only named with alpha-numeric text and a few other items. - * - * @param string $str Input string - * @param bool $fatal Whether to terminate script exection - * or to return FALSE if an invalid - * key is encountered - * @return string|bool - */ - protected function _clean_input_keys($str, $fatal = TRUE) - { - if ( ! preg_match('/^[a-z0-9:_\/|-]+$/i', $str)) - { - if ($fatal === TRUE) - { - return FALSE; - } - else - { - set_status_header(503); - echo 'Disallowed Key Characters.'; - exit(7); // EXIT_USER_INPUT - } - } - - // Clean UTF-8 if supported - if (UTF8_ENABLED === TRUE) - { - return $this->uni->clean_string($str); - } - - return $str; - } - - // -------------------------------------------------------------------- - /** * Request Headers * diff --git a/tests/mocks/core/input.php b/tests/mocks/core/input.php index 40e27441f..4d217a252 100644 --- a/tests/mocks/core/input.php +++ b/tests/mocks/core/input.php @@ -11,16 +11,10 @@ class Mock_Core_Input extends CI_Input { */ public function __construct($security, $utf8) { - $this->_allow_get_array = (config_item('allow_get_array') === TRUE); - $this->_enable_xss = (config_item('global_xss_filtering') === TRUE); $this->_enable_csrf = (config_item('csrf_protection') === TRUE); // Assign Security and Utf8 classes $this->security = $security; - $this->uni = $utf8; - - // Sanitize global arrays - $this->_sanitize_globals(); } public function fetch_from_array($array, $index = '', $xss_clean = FALSE) @@ -28,16 +22,6 @@ class Mock_Core_Input extends CI_Input { return parent::_fetch_from_array($array, $index, $xss_clean); } - /** - * Lie about being a CLI request - * - * We take advantage of this in libraries/Session_test - */ - public function is_cli_request() - { - return FALSE; - } - public function __set($name, $value) { if ($name === 'ip_address') @@ -45,5 +29,4 @@ class Mock_Core_Input extends CI_Input { $this->ip_address = $value; } } - -} \ No newline at end of file +} diff --git a/user_guide_src/source/installation/upgrade_320.rst b/user_guide_src/source/installation/upgrade_320.rst index 6501f40db..8434172e7 100644 --- a/user_guide_src/source/installation/upgrade_320.rst +++ b/user_guide_src/source/installation/upgrade_320.rst @@ -159,3 +159,21 @@ CodeIgniter versions that have been removed in 3.2.0: - ``send_email()`` (use ``mail()`` instead) - The entire *Smiley Helper* (an archived version is available on GitHub: `bcit-ci/ci3-smiley-helper `_) + +Step 8: Make sure you're validating all user inputs +=================================================== + +The :doc:`Input Library <../libraries/input>` used to (often +unconditionally) filter and/or sanitize user input in the ``$_GET``, +``$_POST`` and ``$_COOKIE`` superglobals. + +This was a legacy feature from older times, when things like +`register_globals `_ and +`magic_quotes_gpc `_ existed in +PHP. +It was a necessity back then, but this is no longer the case and reliance +on global filters is a bad practice, giving you a false sense of security. + +This functionality is now removed, and so if you've relied on it for +whatever reasons, you should double-check that you are properly validating +all user inputs in your application (as you always should do). diff --git a/user_guide_src/source/libraries/input.rst b/user_guide_src/source/libraries/input.rst index 1961e3e57..97460c2c5 100644 --- a/user_guide_src/source/libraries/input.rst +++ b/user_guide_src/source/libraries/input.rst @@ -2,10 +2,8 @@ Input Class ########### -The Input Class serves two purposes: - -#. It pre-processes global input data for security. -#. It provides some helper methods for fetching input data and pre-processing it. +The Input Class provides some helper methods for accessing input data +and pre-processing it. .. note:: This class is initialized automatically by the system so there is no need to do it manually. @@ -17,25 +15,9 @@ The Input Class serves two purposes:
-*************** -Input Filtering -*************** - -Security Filtering -================== - -The security filtering method is called automatically when a new -:doc:`controller <../general/controllers>` is invoked. It does the -following: - -- Destroys all global variables in the event register_globals is - turned on. -- Filters the GET/POST/COOKIE array keys, permitting only alpha-numeric - (and a few other) characters. - -******************* -Accessing form data -******************* +******************** +Accessing input data +******************** Using POST, GET, COOKIE, or SERVER Data ======================================= -- cgit v1.2.3-24-g4f1b