From dabddf2154ab7e7703740205a069202554089248 Mon Sep 17 00:00:00 2001 From: rhertogh Date: Sat, 16 Dec 2023 16:11:39 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + src/OAuth1.php | 2 +- src/OAuth2.php | 6 +++++- src/OpenIdConnect.php | 6 +++++- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e10f4d8..b53950c 100644 --- 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 index e2fc63e..8074c6c 100644 --- 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 index e37b4b9..137d008 100644 --- 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 index ca4d1fc..126e986 100644 --- 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');