diff options
author | Florian Pritz <bluewind@xinu.at> | 2016-10-07 12:03:39 +0200 |
---|---|---|
committer | Florian Pritz <bluewind@xinu.at> | 2016-11-05 19:44:15 +0100 |
commit | b654dfa1385ea30827e714add1b3d6944e1ff340 (patch) | |
tree | 251da2937e1fabe9924b8394df359e14c769926f | |
parent | 63db15c7d1aad2d115163621e0fa7d88960314ac (diff) |
MY_Controller: Extract CSRF code into method
Signed-off-by: Florian Pritz <bluewind@xinu.at>
-rw-r--r-- | application/core/MY_Controller.php | 65 |
1 files changed, 37 insertions, 28 deletions
diff --git a/application/core/MY_Controller.php b/application/core/MY_Controller.php index afd8b5ef2..47dd6a899 100644 --- a/application/core/MY_Controller.php +++ b/application/core/MY_Controller.php @@ -16,7 +16,6 @@ class MY_Controller extends CI_Controller { parent::__construct(); $this->var = new StdClass(); - $csrf_protection = true; $this->load->library('customautoloader'); @@ -35,33 +34,7 @@ class MY_Controller extends CI_Controller { is_api_client(true); } - if ($this->input->post("apikey") !== false || is_api_client()) { - /* This relies on the authentication code always verifying the supplied - * apikey. If the key is not verified/logged in an attacker could simply - * add an empty "apikey" field to the CSRF form to circumvent the - * protection. If we always log in if a key is supplied we can ensure - * that an attacker (and the victim since they get a cookie) can only - * access the attacker's account. - */ - $csrf_protection = false; - } - - $uri_start = $this->uri->rsegment(1)."/".$this->uri->rsegment(2); - $csrf_whitelisted_handlers = array( - "always" => array( - /* Whitelist the upload pages because they don't cause harm and a user - * might keep the upload page open for more than csrf_expire seconds - * and we don't want to annoy them when they upload a big file and the - * CSRF check fails. - */ - "file/do_websubmit", - ), - ); - if (in_array($uri_start, $csrf_whitelisted_handlers["always"])) { - $csrf_protection = false; - } - - if ($csrf_protection && !$this->input->is_cli_request()) { + if ($this->_check_csrf_protection_required()) { $this->_setup_csrf_protection(); } @@ -104,6 +77,42 @@ class MY_Controller extends CI_Controller { } } + private function _check_csrf_protection_required() + { + if ($this->input->post("apikey") !== false || is_api_client()) { + /* This relies on the authentication code always verifying the supplied + * apikey. If the key is not verified/logged in an attacker could simply + * add an empty "apikey" field to the CSRF form to circumvent the + * protection. If we always log in if a key is supplied we can ensure + * that an attacker (and the victim since they get a cookie) can only + * access the attacker's account. + */ + // TODO: perform the apikey login here to make sure this works as expected? + return false; + } + + $uri_start = $this->uri->rsegment(1)."/".$this->uri->rsegment(2); + $csrf_whitelisted_handlers = array( + "always" => array( + /* Whitelist the upload pages because they don't cause harm and a user + * might keep the upload page open for more than csrf_expire seconds + * and we don't want to annoy them when they upload a big file and the + * CSRF check fails. + */ + "file/do_websubmit", + ), + ); + if (in_array($uri_start, $csrf_whitelisted_handlers["always"])) { + return false; + } + + if ($this->input->is_cli_request()) { + return false; + } + + return true; + } + private function _setup_csrf_protection() { // 2 functions for accessing config options, really? |