Merge pull request from GHSA-w8vh-p74j-x9xp

* Fixed Oauth1/2 and OpenID Connect state/noce check * Updated changelog for GHSA-w8vh-p74j-x9xp --------- Co-authored-by: Alexander Makarov <[email protected]>
yiisoft · Dec 16, 2023 · dabddf2 · dabddf2
1 parent 721ed97
commit dabddf2
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@ Yii Framework 2 authclient extension Change Log
 
 - Bug #364: Use issuer claim from OpenID Configuration (radwouters)
 - Enh #367: Throw more specific `ClientErrorResponseException` when the response code in `BaseOAuth::sendRequest()` is a 4xx (rhertogh)
+- Enh GHSA-w8vh-p74j-x9xp: Improved security for OAuth1, OAuth2 and OpenID Connect clients by using timing attack safe string comparsion (rhertogh)
 - Enh GHSA-rw54-6826-c8j5: Improved security for OAuth2 client by requiring an `authCodeVerifier` if PKCE is enabled and clearing it after usage (rhertogh)
 
 

diff --git a/src/OAuth1.php b/src/OAuth1.php
@@ -155,7 +155,7 @@ public function fetchAccessToken($oauthToken = null, OAuthToken $requestToken =
             }
         }
 
-        if (strcmp($requestToken->getToken(), $oauthToken) !== 0) {
+        if (!Yii::$app->getSecurity()->compareString($requestToken->getToken(), $oauthToken)) {
             throw new HttpException(400, 'Invalid auth state parameter.');
         }
 

diff --git a/src/OAuth2.php b/src/OAuth2.php
@@ -118,7 +118,11 @@ public function fetchAccessToken($authCode, array $params = [])
             $authState = $this->getState('authState');
             $incomingRequest = Yii::$app->getRequest();
             $incomingState = $incomingRequest->get('state', $incomingRequest->post('state'));
-            if (!isset($incomingState) || empty($authState) || strcmp($incomingState, $authState) !== 0) {
+            if (
+                !isset($incomingState)
+                || empty($authState)
+                || !Yii::$app->getSecurity()->compareString($incomingState, $authState)
+            ) {
                 throw new HttpException(400, 'Invalid auth state parameter.');
             }
             $this->removeState('authState');

diff --git a/src/OpenIdConnect.php b/src/OpenIdConnect.php
@@ -417,7 +417,11 @@ protected function createToken(array $tokenConfig = [])
 
             if ($this->getValidateAuthNonce()) {
                 $authNonce = $this->getState('authNonce');
-                if (!isset($jwsData['nonce']) || empty($authNonce) || strcmp($jwsData['nonce'], $authNonce) !== 0) {
+                if (
+                    !isset($jwsData['nonce'])
+                    || empty($authNonce)
+                    || !Yii::$app->getSecurity()->compareString($jwsData['nonce'], $authNonce)
+                ) {
                     throw new HttpException(400, 'Invalid auth nonce');
                 } else {
                     $this->removeState('authNonce');
-Original file line number
+Diff line change
@@ Expand Up @@
                 }
             }
-            if (strcmp($requestToken->getToken(), $oauthToken) !== 0) {
+            if (!Yii::$app->getSecurity()->compareString($requestToken->getToken(), $oauthToken)) {
                 throw new HttpException(400, 'Invalid auth state parameter.');
             }
@@ Expand Down @@