Catch errors thrown by "decodeURIComponent" (#6226)

- Catch errors thrown by "decodeURIComponent" - Negate state condition check for better readability.
AzureAD · Jul 10, 2023 · 251b39a · 251b39a
1 parent 1045be1
commit 251b39a
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 7 deletions.
diff --git a/change/@azure-msal-common-334d43a0-8c93-4c68-bb48-1debb1510815.json b/change/@azure-msal-common-334d43a0-8c93-4c68-bb48-1debb1510815.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "Catch errors thrown by \"decodeURIComponent\" #6226",
+  "packageName": "@azure/msal-common",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/lib/msal-common/src/response/ResponseHandler.ts b/lib/msal-common/src/response/ResponseHandler.ts
@@ -82,15 +82,33 @@ export class ResponseHandler {
         cryptoObj: ICrypto
     ): void {
         if (!serverResponseHash.state || !cachedState) {
-            throw !serverResponseHash.state
-                ? ClientAuthError.createStateNotFoundError("Server State")
-                : ClientAuthError.createStateNotFoundError("Cached State");
+            throw serverResponseHash.state
+                ? ClientAuthError.createStateNotFoundError("Cached State")
+                : ClientAuthError.createStateNotFoundError("Server State");
         }
 
-        if (
-            decodeURIComponent(serverResponseHash.state) !==
-            decodeURIComponent(cachedState)
-        ) {
+        let decodedServerResponseHash: string;
+        let decodedCachedState: string;
+
+        try {
+            decodedServerResponseHash = decodeURIComponent(serverResponseHash.state);
+        } catch (e) {
+            throw ClientAuthError.createInvalidStateError(
+                serverResponseHash.state,
+                `Server response hash URI could not be decoded`
+            );
+        }
+
+        try {
+            decodedCachedState = decodeURIComponent(cachedState);
+        } catch (e) {
+            throw ClientAuthError.createInvalidStateError(
+                serverResponseHash.state,
+                `Cached state URI could not be decoded`
+            );
+        }
+
+        if (decodedServerResponseHash !== decodedCachedState) {
             throw ClientAuthError.createStateMismatchError();
         }
 

diff --git a/lib/msal-common/test/response/ResponseHandler.spec.ts b/lib/msal-common/test/response/ResponseHandler.spec.ts
@@ -847,5 +847,35 @@ describe("ResponseHandler.ts", () => {
             );
             expect(buildClientInfoSpy.notCalled).toBe(true);
         });
+
+        it("throws invalid state error", (done) => {
+            const testServerCodeResponse: ServerAuthorizationCodeResponse = {
+                code: "testCode",
+                client_info: TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO,
+                state: TEST_STATE_VALUES.URI_ENCODED_LIB_STATE,
+            };
+
+            const responseHandler = new ResponseHandler(
+                "this-is-a-client-id",
+                testCacheManager,
+                cryptoInterface,
+                logger,
+                null,
+                null
+            );
+
+            try {
+                responseHandler.validateServerAuthorizationCodeResponse(
+                    testServerCodeResponse,
+                    'dummy-state-%20%%%30%%%%%40',
+                    cryptoInterface
+                );
+            } catch (e) {
+                expect(e).toBeInstanceOf(ClientAuthError);
+                const err = e as ClientAuthError;
+                expect(err.message).toContain(`Cached state URI could not be decoded`);
+                done();
+            }
+        });
     });
 });