From 9f20c8011a80d74edb740081cd96388bb6a967e6 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Wed, 14 Dec 2016 18:41:52 +0200 Subject: Move csrf_verify() call out of CI_Input --- system/core/CodeIgniter.php | 2 +- system/core/Input.php | 20 +---------- system/core/Security.php | 9 ++--- tests/codeigniter/core/Input_test.php | 40 ++++++++++++---------- tests/codeigniter/core/Security_test.php | 5 +-- tests/codeigniter/helpers/cookie_helper_test.php | 4 +-- tests/codeigniter/helpers/security_helper_test.php | 2 +- .../codeigniter/libraries/Form_validation_test.php | 4 +-- tests/codeigniter/libraries/Session_test.php | 5 +-- tests/codeigniter/libraries/Upload_test.php | 2 +- tests/mocks/core/input.php | 30 ---------------- 11 files changed, 41 insertions(+), 82 deletions(-) delete mode 100644 tests/mocks/core/input.php diff --git a/system/core/CodeIgniter.php b/system/core/CodeIgniter.php index 410b9613b..977d1427d 100644 --- a/system/core/CodeIgniter.php +++ b/system/core/CodeIgniter.php @@ -281,7 +281,7 @@ defined('BASEPATH') OR exit('No direct script access allowed'); * Load the security class for xss and csrf support * ----------------------------------------------------- */ - $SEC =& load_class('Security', 'core'); + $SEC =& load_class('Security', 'core', $charset); /* * ------------------------------------------------------ diff --git a/system/core/Input.php b/system/core/Input.php index d881e253d..ab60e45c3 100644 --- a/system/core/Input.php +++ b/system/core/Input.php @@ -57,16 +57,6 @@ class CI_Input { */ protected $ip_address = FALSE; - /** - * Enable CSRF flag - * - * Enables a CSRF cookie token to be set. - * Set automatically based on config setting. - * - * @var bool - */ - protected $_enable_csrf = FALSE; - /** * List of all HTTP request headers * @@ -115,15 +105,7 @@ class CI_Input { */ public function __construct(CI_Security &$security) { - $this->_enable_csrf = (config_item('csrf_protection') === TRUE); - $this->security = $security; - - // CSRF Protection check - if ($this->_enable_csrf === TRUE && ! is_cli()) - { - $this->security->csrf_verify(); - } - + $this->security = $security; log_message('info', 'Input Class Initialized'); } diff --git a/system/core/Security.php b/system/core/Security.php index a80b52fd1..fb0ca3d4e 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -167,10 +167,12 @@ class CI_Security { * * @return void */ - public function __construct() + public function __construct($charset) { + $this->charset = $charset; + // Is CSRF protection enabled? - if (config_item('csrf_protection')) + if (config_item('csrf_protection') && ! is_cli()) { // CSRF config foreach (array('csrf_expire', 'csrf_token_name', 'csrf_cookie_name') as $key) @@ -189,10 +191,9 @@ class CI_Security { // Set the CSRF hash $this->_csrf_set_hash(); + $this->csrf_verify(); } - $this->charset = strtoupper(config_item('charset')); - log_message('info', 'Security Class Initialized'); } diff --git a/tests/codeigniter/core/Input_test.php b/tests/codeigniter/core/Input_test.php index 78b659691..e068a84be 100644 --- a/tests/codeigniter/core/Input_test.php +++ b/tests/codeigniter/core/Input_test.php @@ -12,10 +12,8 @@ class Input_test extends CI_TestCase { $this->ci_set_config('global_xss_filtering', FALSE); $this->ci_set_config('csrf_protection', FALSE); - $security = new Mock_Core_Security(); - - $this->ci_set_config('charset', 'UTF-8'); - $this->input = new Mock_Core_Input($security); + $security = new Mock_Core_Security('UTF-8'); + $this->input = new CI_Input($security); } // -------------------------------------------------------------------- @@ -120,14 +118,17 @@ class Input_test extends CI_TestCase { public function test_fetch_from_array() { + $reflection = new ReflectionMethod($this->input, '_fetch_from_array'); + $reflection->setAccessible(TRUE); + $data = array( 'foo' => 'bar', 'harm' => 'Hello, i try to your site', ); - $foo = $this->input->fetch_from_array($data, 'foo'); - $harm = $this->input->fetch_from_array($data, 'harm'); - $harmless = $this->input->fetch_from_array($data, 'harm', TRUE); + $foo = $reflection->invokeArgs($this->input, [&$data, 'foo']); + $harm = $reflection->invokeArgs($this->input, [&$data, 'harm']); + $harmless = $reflection->invokeArgs($this->input, [&$data, 'harm', TRUE]); $this->assertEquals('bar', $foo); $this->assertEquals("Hello, i try to your site", $harm); @@ -215,57 +216,60 @@ class Input_test extends CI_TestCase { public function test_ip_address() { - $this->input->ip_address = '127.0.0.1'; + $reflection = new ReflectionProperty($this->input, 'ip_address'); + $reflection->setAccessible(TRUE); + + $reflection->setValue($this->input, '127.0.0.1'); $this->assertEquals('127.0.0.1', $this->input->ip_address()); // 127.0.0.1 is set in our Bootstrap file - $this->input->ip_address = FALSE; + $reflection->setValue($this->input, FALSE); $this->assertEquals('127.0.0.1', $this->input->ip_address()); // Invalid $_SERVER['REMOTE_ADDR'] = 'invalid_ip_address'; - $this->input->ip_address = FALSE; // reset cached value + $reflection->setValue($this->input, FALSE); // reset cached value $this->assertEquals('0.0.0.0', $this->input->ip_address()); $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; // Proxy_ips tests - $this->input->ip_address = FALSE; + $reflection->setValue($this->input, FALSE); $this->ci_set_config('proxy_ips', '127.0.0.3, 127.0.0.4, 127.0.0.2'); $_SERVER['HTTP_CLIENT_IP'] = '127.0.0.2'; $this->assertEquals('127.0.0.1', $this->input->ip_address()); // Invalid spoof - $this->input->ip_address = FALSE; + $reflection->setValue($this->input, FALSE); $this->ci_set_config('proxy_ips', 'invalid_ip_address'); $_SERVER['HTTP_CLIENT_IP'] = 'invalid_ip_address'; $this->assertEquals('127.0.0.1', $this->input->ip_address()); - $this->input->ip_address = FALSE; + $reflection->setValue($this->input, FALSE); $this->ci_set_config('proxy_ips', 'http://foo/bar/baz, 127.0.0.1/1'); $_SERVER['HTTP_CLIENT_IP'] = '127.0.0.1'; $this->assertEquals('127.0.0.1', $this->input->ip_address()); - $this->input->ip_address = FALSE; + $reflection->setValue($this->input, FALSE); $this->ci_set_config('proxy_ips', 'http://foo/bar/baz, 127.0.0.2'); $_SERVER['HTTP_CLIENT_IP'] = '127.0.0.2'; $_SERVER['REMOTE_ADDR'] = '127.0.0.2'; $this->assertEquals('127.0.0.2', $this->input->ip_address()); - //IPv6 - $this->input->ip_address = FALSE; + // IPv6 + $reflection->setValue($this->input, FALSE); $this->ci_set_config('proxy_ips', 'FE80:0000:0000:0000:0202:B3FF:FE1E:8329/1, FE80:0000:0000:0000:0202:B3FF:FE1E:8300/2'); $_SERVER['HTTP_CLIENT_IP'] = 'FE80:0000:0000:0000:0202:B3FF:FE1E:8300'; $_SERVER['REMOTE_ADDR'] = 'FE80:0000:0000:0000:0202:B3FF:FE1E:8329'; $this->assertEquals('FE80:0000:0000:0000:0202:B3FF:FE1E:8300', $this->input->ip_address()); - $this->input->ip_address = FALSE; + $reflection->setValue($this->input, FALSE); $this->ci_set_config('proxy_ips', '0::/32'); $_SERVER['HTTP_CLIENT_IP'] = '127.0.0.7'; $_SERVER['REMOTE_ADDR'] = '0000:0000:0000:0000:0000:0000:0000:0001'; $this->assertEquals('127.0.0.7', $this->input->ip_address()); - $this->input->ip_address = FALSE; + $reflection->setValue($this->input, FALSE); $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; // back to reality } diff --git a/tests/codeigniter/core/Security_test.php b/tests/codeigniter/core/Security_test.php index cbf0285ec..2e1127f87 100644 --- a/tests/codeigniter/core/Security_test.php +++ b/tests/codeigniter/core/Security_test.php @@ -12,7 +12,8 @@ class Security_test extends CI_TestCase { $this->ci_set_config('csrf_token_name', 'ci_csrf_token'); $this->ci_set_config('csrf_cookie_name', 'ci_csrf_cookie'); - $this->security = new Mock_Core_Security(); + $_SERVER['REQUEST_METHOD'] = 'GET'; + $this->security = new Mock_Core_Security('UTF-8'); } // -------------------------------------------------------------------- @@ -341,7 +342,7 @@ class Security_test extends CI_TestCase { // leave csrf_cookie_name as blank to test _csrf_set_hash function $this->ci_set_config('csrf_cookie_name', ''); - $this->security = new Mock_Core_Security(); + $this->security = new Mock_Core_Security('UTF-8'); $this->assertNotEmpty($this->security->get_csrf_hash()); } diff --git a/tests/codeigniter/helpers/cookie_helper_test.php b/tests/codeigniter/helpers/cookie_helper_test.php index 1fbb57f67..e984be21c 100644 --- a/tests/codeigniter/helpers/cookie_helper_test.php +++ b/tests/codeigniter/helpers/cookie_helper_test.php @@ -28,9 +28,9 @@ class Cookie_helper_test extends CI_TestCase { { $_COOKIE['foo'] = 'bar'; - $security = new Mock_Core_Security(); + $security = new Mock_Core_Security('UTF-8'); $input_cls = $this->ci_core_class('input'); - $this->ci_instance_var('input', new Mock_Core_Input($security)); + $this->ci_instance_var('input', new CI_Input($security)); $this->assertEquals('bar', get_cookie('foo', FALSE)); $this->assertEquals('bar', get_cookie('foo', TRUE)); diff --git a/tests/codeigniter/helpers/security_helper_test.php b/tests/codeigniter/helpers/security_helper_test.php index ab05d57ba..d7e3f4734 100644 --- a/tests/codeigniter/helpers/security_helper_test.php +++ b/tests/codeigniter/helpers/security_helper_test.php @@ -6,7 +6,7 @@ class Security_helper_tests extends CI_TestCase { { $this->helper('security'); $obj = new stdClass; - $obj->security = new Mock_Core_Security(); + $obj->security = new Mock_Core_Security('UTF-8'); $this->ci_instance($obj); } diff --git a/tests/codeigniter/libraries/Form_validation_test.php b/tests/codeigniter/libraries/Form_validation_test.php index 5b7830dd8..edbe9da4a 100644 --- a/tests/codeigniter/libraries/Form_validation_test.php +++ b/tests/codeigniter/libraries/Form_validation_test.php @@ -13,8 +13,8 @@ class Form_validation_test extends CI_TestCase { // Same applies for lang $lang = $this->getMockBuilder('CI_Lang')->setMethods(array('load'))->getMock(); - $security = new Mock_Core_Security(); - $input = new Mock_Core_Input($security); + $security = new Mock_Core_Security('UTF-8'); + $input = new CI_Input($security); $this->ci_instance_var('lang', $lang); $this->ci_instance_var('load', $loader); diff --git a/tests/codeigniter/libraries/Session_test.php b/tests/codeigniter/libraries/Session_test.php index 76a4fcc98..840df076a 100644 --- a/tests/codeigniter/libraries/Session_test.php +++ b/tests/codeigniter/libraries/Session_test.php @@ -37,7 +37,8 @@ return; $ci = $this->ci_instance(); $ldr = $this->ci_core_class('load'); $ci->load = new $ldr(); - $ci->input = new Mock_Core_Input(NULL, NULL); + $security = new Mock_Core_Security('UTF-8'); + $ci->input = new CI_Input($security); // Make sure string helper is available $this->ci_vfs_clone('system/helpers/string_helper.php'); @@ -437,4 +438,4 @@ return; $this->assertNull($this->session->native->userdata($key)); } -} \ No newline at end of file +} diff --git a/tests/codeigniter/libraries/Upload_test.php b/tests/codeigniter/libraries/Upload_test.php index 8bac597b3..74a7d2c22 100644 --- a/tests/codeigniter/libraries/Upload_test.php +++ b/tests/codeigniter/libraries/Upload_test.php @@ -6,7 +6,7 @@ class Upload_test extends CI_TestCase { { $ci = $this->ci_instance(); $ci->upload = new CI_Upload(); - $ci->security = new Mock_Core_Security(); + $ci->security = new Mock_Core_Security('UTF-8'); $ci->lang = $this->getMockBuilder('CI_Lang')->setMethods(array('load', 'line'))->getMock(); $ci->lang->expects($this->any())->method('line')->will($this->returnValue(FALSE)); $this->upload = $ci->upload; diff --git a/tests/mocks/core/input.php b/tests/mocks/core/input.php deleted file mode 100644 index 6f6a91365..000000000 --- a/tests/mocks/core/input.php +++ /dev/null @@ -1,30 +0,0 @@ -_enable_csrf = (config_item('csrf_protection') === TRUE); - $this->security = $security; - } - - public function fetch_from_array($array, $index = '', $xss_clean = FALSE) - { - return parent::_fetch_from_array($array, $index, $xss_clean); - } - - public function __set($name, $value) - { - if ($name === 'ip_address') - { - $this->ip_address = $value; - } - } -} -- cgit v1.2.3-24-g4f1b