SEC-7244: Setting new cookie i.e expiry time along with csrf token. (#6)

* Adding new cookie i.r expiry time along with token * Update test cases.
theorchard · Apr 3, 2023 · e1bdd99 · e1bdd99
1 parent 87b133d
commit e1bdd99
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 9 deletions.
diff --git a/libs/csrf/csrfprotector.php b/libs/csrf/csrfprotector.php
@@ -8,6 +8,7 @@
 
     // name of HTTP POST variable for authentication
     define("CSRFP_TOKEN","csrfp_token");
+    define("CSRFP_TOKEN_EXPIRY","csrfp_token_expiry");
 
     // We insert token name and list of url patterns for which
     // GET requests are validated against CSRF as hidden input fields
@@ -158,6 +159,9 @@ public static function init($length = null, $action = null, $logger = null)
             if (self::$config['CSRFP_TOKEN'] == '')
                 self::$config['CSRFP_TOKEN'] = CSRFP_TOKEN;
 
+            if (self::$config['CSRFP_TOKEN_EXPIRY'] == '')
+                self::$config['CSRFP_TOKEN_EXPIRY'] = CSRFP_TOKEN_EXPIRY;
+
             self::$tokenHeaderKey = 'HTTP_' .strtoupper(self::$config['CSRFP_TOKEN']);
             self::$tokenHeaderKey = str_replace('-', '_', self::$tokenHeaderKey);
 
@@ -297,13 +301,18 @@ private static function getTokenFromRequest() {
         private static function isValidToken($token) {
             if (!isset($_SESSION[self::$config['CSRFP_TOKEN']])) return false;
             if (!is_array($_SESSION[self::$config['CSRFP_TOKEN']])) return false;
-            // Clear match token from the session
-            foreach ($_SESSION[self::$config['CSRFP_TOKEN']] as $_key => $_value) {
-                if ($_value == $token) {
-                    unset($_SESSION[self::$config['CSRFP_TOKEN']][$_key]);
+            foreach ($_SESSION[self::$config['CSRFP_TOKEN']] as $key => $value) {
+                if ($value == $token) {
+
+                    // Clear all older tokens assuming they have been consumed
+                    foreach ($_SESSION[self::$config['CSRFP_TOKEN']] as $_key => $_value) {
+                        if ($_value == $token) break;
+                        array_shift($_SESSION[self::$config['CSRFP_TOKEN']]);
+                    }
                     return true;
                 }
             }
+
             return false;
         }
 
@@ -393,10 +402,20 @@ public static function refreshToken()
                 self::$cookieConfig = new csrfpCookieConfig(self::$config['cookieConfig']);
             }
 
+            $expiryTime = time() + self::$cookieConfig->expire;
+
             setcookie(
                 self::$config['CSRFP_TOKEN'], 
                 $token,
-                time() + self::$cookieConfig->expire,
+                $expiryTime,
+                self::$cookieConfig->path,
+                self::$cookieConfig->domain,
+                (bool) self::$cookieConfig->secure);
+
+            setcookie(
+                self::$config['CSRFP_TOKEN_EXPIRY'],
+                $expiryTime,
+                $expiryTime,
                 self::$cookieConfig->path,
                 self::$cookieConfig->domain,
                 (bool) self::$cookieConfig->secure);

diff --git a/test/config.test.php b/test/config.test.php
@@ -9,6 +9,7 @@
  */
 return array(
 	"CSRFP_TOKEN" => "csrfp_token",
+	"CSRFP_TOKEN_EXPIRY" => "csrfp_token_expiry",
 	"logDirectory" => "../log",
 	"failedAuthAction" => array(
 		"GET" => 0,

diff --git a/test/config.testInit_incompleteConfigurationException.php b/test/config.testInit_incompleteConfigurationException.php
@@ -9,6 +9,7 @@
  */
 return array(
 	"CSRFP_TOKEN" => "csrfp_token",
+	"CSRFP_TOKEN_EXPIRY" => "csrfp_token_expiry",
 //	"logDirectory" => "../log",
 //	"failedAuthAction" => array(
 //		"GET" => 0,

diff --git a/test/config.testInit_withoutInjectedCSRFGuardScript.php b/test/config.testInit_withoutInjectedCSRFGuardScript.php
@@ -9,6 +9,7 @@
  */
 return array(
 	"CSRFP_TOKEN" => "csrfp_token",
+	"CSRFP_TOKEN_EXPIRY" => "csrfp_token_expiry",
 	"logDirectory" => "../log",
 	"failedAuthAction" => array(
 		"GET" => 0,

diff --git a/test/csrfprotector_test.php b/test/csrfprotector_test.php
@@ -97,6 +97,7 @@ public function setUp()
         $this->logDir = __DIR__ .'/logs';
 
         csrfprotector::$config['CSRFP_TOKEN'] = 'csrfp_token';
+        csrfprotector::$config['CSRFP_TOKEN_EXPIRY'] = 'csrfp_token_expiry';
         csrfprotector::$config['cookieConfig'] = array('secure' => false);
         csrfprotector::$config['logDirectory'] = '../test/logs';
 
@@ -396,7 +397,7 @@ public function testAuthorisePost_success()
         $temp = $_SESSION[csrfprotector::$config['CSRFP_TOKEN']];
 
         csrfprotector::authorizePost(); //will create new session and cookies
-        $this->assertTrue(!isset($_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0]));
+        $this->assertFalse($temp == $_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0]);
         $this->assertTrue(csrfp_wrapper::checkHeader('Set-Cookie'));
         $this->assertTrue(csrfp_wrapper::checkHeader('csrfp_token'));
         // $this->assertTrue(csrfp_wrapper::checkHeader($_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0]));  // Combine these 3 later
@@ -406,7 +407,7 @@ public function testAuthorisePost_success()
         csrfp_wrapper::changeRequestType('GET');
         $_POST[csrfprotector::$config['CSRFP_TOKEN']]
             = $_GET[csrfprotector::$config['CSRFP_TOKEN']]
-            = $_SESSION[csrfprotector::$config['CSRFP_TOKEN']][1];
+            = $_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0];
         $temp = $_SESSION[csrfprotector::$config['CSRFP_TOKEN']];
 
         csrfprotector::authorizePost(); //will create new session and cookies
@@ -437,7 +438,7 @@ public function testAuthorisePost_success_2()
         $temp = $_SESSION[csrfprotector::$config['CSRFP_TOKEN']];
 
         csrfprotector::authorizePost(); //will create new session and cookies
-        $this->assertTrue(!isset($_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0]));
+        $this->assertFalse($temp == $_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0]);
         $this->assertTrue(csrfp_wrapper::checkHeader('Set-Cookie'));
         $this->assertTrue(csrfp_wrapper::checkHeader('csrfp_token'));
         // $this->assertTrue(csrfp_wrapper::checkHeader($_SESSION[csrfprotector::$config['CSRFP_TOKEN']][0]));  // Combine these 3 later
@@ -733,7 +734,7 @@ public function testMultipleInitializeException()
         $_SERVER['REQUEST_METHOD'] = 'GET';
         csrfProtector::init();
 
-        $this->assertTrue(count(csrfProtector::$config) == 10);
+        $this->assertTrue(count(csrfProtector::$config) == 11);
         try {
             csrfProtector::init();
             $this->fail("alreadyInitializedException not raised");

diff --git a/test/csrfprotector_test_customlogger.php b/test/csrfprotector_test_customlogger.php
@@ -25,6 +25,7 @@ class csrfp_test_customLogger extends PHPUnit_Framework_TestCase
 
     public function setUp() {
         csrfprotector::$config['CSRFP_TOKEN'] = 'csrfp_token';
+        csrfprotector::$config['CSRFP_TOKEN_EXPIRY'] = 'csrfp_token_expiry';
         csrfprotector::$config['cookieConfig'] = array('secure' => false);
         csrfprotector::$config['logDirectory'] = '../test/logs';
         $_SERVER['REQUEST_URI'] = 'temp';       // For logging