Backport security fixes

narfbg · narfbg · commit 481e42660f3c · 2012-10-06T15:42:56.000+03:00
diff --git a/system/core/Security.php b/system/core/Security.php
@@ -98,26 +98,32 @@ class CI_Security {
 
 	/**
 	 * Constructor
+	 *
+	 * @return	void
 	 */
 	public function __construct()
 	{
-		// CSRF config
-		foreach(array('csrf_expire', 'csrf_token_name', 'csrf_cookie_name') as $key)
+		// Is CSRF protection enabled?
+		if (config_item('csrf_protection') === TRUE)
 		{
-			if (FALSE !== ($val = config_item($key)))
+			// CSRF config
+			foreach (array('csrf_expire', 'csrf_token_name', 'csrf_cookie_name') as $key)
 			{
-				$this->{'_'.$key} = $val;
+				if (FALSE !== ($val = config_item($key)))
+				{
+					$this->{'_'.$key} = $val;
+				}
 			}
-		}
 
-		// Append application specific cookie prefix
-		if (config_item('cookie_prefix'))
-		{
-			$this->_csrf_cookie_name = config_item('cookie_prefix').$this->_csrf_cookie_name;
-		}
+			// Append application specific cookie prefix
+			if (config_item('cookie_prefix'))
+			{
+				$this->_csrf_cookie_name = config_item('cookie_prefix').$this->_csrf_cookie_name;
+			}
 
-		// Set the CSRF hash
-		$this->_csrf_set_hash();
+			// Set the CSRF hash
+			$this->_csrf_set_hash();
+		}
 
 		log_message('debug', "Security Class Initialized");
 	}
@@ -131,15 +137,14 @@ public function __construct()
 	 */
 	public function csrf_verify()
 	{
-		// If no POST data exists we will set the CSRF cookie
-		if (count($_POST) == 0)
+		// If it's not a POST request we will set the CSRF cookie
+		if (strtoupper($_SERVER['REQUEST_METHOD']) !== 'POST')
 		{
 			return $this->csrf_set_cookie();
 		}
 
 		// Do the tokens exist in both the _POST and _COOKIE arrays?
-		if ( ! isset($_POST[$this->_csrf_token_name]) OR
-			 ! isset($_COOKIE[$this->_csrf_cookie_name]))
+		if ( ! isset($_POST[$this->_csrf_token_name], $_COOKIE[$this->_csrf_cookie_name]))
 		{
 			$this->csrf_show_error();
 		}
@@ -159,7 +164,7 @@ public function csrf_verify()
 		$this->_csrf_set_hash();
 		$this->csrf_set_cookie();
 
-		log_message('debug', "CSRF token verified ");
+		log_message('debug', 'CSRF token verified');
 
 		return $this;
 	}
@@ -176,14 +181,9 @@ public function csrf_set_cookie()
 		$expire = time() + $this->_csrf_expire;
 		$secure_cookie = (config_item('cookie_secure') === TRUE) ? 1 : 0;
 
-		if ($secure_cookie)
+		if ($secure_cookie && (empty($_SERVER['HTTPS']) OR strtolower($_SERVER['HTTPS']) === 'off'))
 		{
-			$req = isset($_SERVER['HTTPS']) ? $_SERVER['HTTPS'] : FALSE;
-
-			if ( ! $req OR $req == 'off')
-			{
-				return FALSE;
-			}
+			return FALSE;
 		}
 
 		setcookie($this->_csrf_cookie_name, $this->_csrf_hash, $expire, config_item('cookie_path'), config_item('cookie_domain'), $secure_cookie);
@@ -871,7 +871,6 @@ protected function _csrf_set_hash()
 	}
 
 }
-// END Security Class
 
 /* End of file Security.php */
-/* Location: ./system/libraries/Security.php */
+/* Location: ./system/libraries/Security.php */
diff --git a/user_guide/changelog.html b/user_guide/changelog.html
@@ -68,6 +68,8 @@ <h3>Bug fixes for 2.1.3:</h3>
 	<li>Fixed a bug (#1699) - <a href="libraries/migration.html">Migration Library</a> ignored the <samp>$config['migration_path']</samp> setting.</li>
 	<li>Fixed a bug (#227) - <a href="libraries/input.html">Input Library</a> allowed unconditional spoofing of HTTP clients' IP addresses through the HTTP_CLIENT_IP header.</li>
 	<li>Fixed a bug (#907) - <a href="libraries/input.html">Input Library</a> ignored HTTP_X_CLUSTER_CLIENT_IP and HTTP_X_CLIENT_IP headers when checking for proxies.</li>
+	<li>Fixed a bug (#940) - <samp>csrf_verify()</samp> used to set the CSRF cookie while processing a POST request with no actual POST data, which resulted in validating a request that should be considered invalid.</li>
+	<li>Fixed a bug in the <a href="libraries/security.html">Security Library</a> where a CSRF cookie was created even if <samp>$config['csrf_protection']</samp> is set tot FALSE.</li>
 </ul>
 
 <h2>Version 2.1.2</h2>
