From 786915852f698f2c77a29b2a33b4cd44190d0c77 Mon Sep 17 00:00:00 2001
From: Florian Pritz <bluewind@xinu.at>
Date: Fri, 7 Oct 2016 12:03:39 +0200
Subject: MY_Controller: Extract CSRF code into method

Signed-off-by: Florian Pritz <bluewind@xinu.at>
---
 application/core/MY_Controller.php | 65 ++++++++++++++++++++++----------------
 1 file 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?
-- 
cgit v1.2.3-24-g4f1b