Guard clientDataJSON members before use

src/CreateResponse.php

@@ -62,13 +62,15 @@ public function verify(
         }
 
         // 7.1.7
-        if ($C['type'] !== 'webauthn.create') {
+        if (!array_key_exists('type', $C) || $C['type'] !== 'webauthn.create') {
             $this->fail('7.1.7', 'C.type');
         }
 
         // 7.1.8
+        if (!array_key_exists('challenge', $C) || !is_string($C['challenge'])) {
+            $this->fail('7.1.8', 'C.challenge');
+        }
         $cdjChallenge = $C['challenge'];
-        assert(is_string($cdjChallenge));
         $challenge = $challengeLoader->useFromClientDataJSON($cdjChallenge);
         if ($challenge === null) {
             $this->fail('7.1.8', 'C.challenge');
@@ -80,8 +82,11 @@ public function verify(
         }
 
         // 7.1.9
-        assert(array_key_exists('origin', $C) && is_string($C['origin']));
-        if (!$rp->matchesOrigin($C['origin'])) {
+        if (
+            !array_key_exists('origin', $C)
+            || !is_string($C['origin'])
+            || !$rp->matchesOrigin($C['origin'])
+        ) {
             $this->fail('7.1.9', 'C.origin');
         }
 

src/GetResponse.php

@@ -7,7 +7,6 @@
 use UnexpectedValueException;
 
 use function array_key_exists;
-use function assert;
 use function is_array;
 use function is_string;
 
@@ -107,13 +106,15 @@ public function verify(
         }
 
         // 7.2.12
-        if ($C['type'] !== 'webauthn.get') {
+        if (!array_key_exists('type', $C) || $C['type'] !== 'webauthn.get') {
             $this->fail('7.2.11', 'C.type');
         }
 
         // 7.2.13
+        if (!array_key_exists('challenge', $C) || !is_string($C['challenge'])) {
+            $this->fail('7.2.12', 'C.challenge');
+        }
         $cdjChallenge = $C['challenge'];
-        assert(is_string($cdjChallenge));
         $challenge = $challengeLoader->useFromClientDataJSON($cdjChallenge);
         if ($challenge === null) {
             $this->fail('7.2.12', 'C.challenge');
@@ -125,8 +126,11 @@ public function verify(
         }
 
         // 7.2.14
-        assert(array_key_exists('origin', $C) && is_string($C['origin']));
-        if (!$rp->matchesOrigin($C['origin'])) {
+        if (
+            !array_key_exists('origin', $C)
+            || !is_string($C['origin'])
+            || !$rp->matchesOrigin($C['origin'])
+        ) {
             $this->fail('7.2.13', 'C.origin');
         }
 

tests/CreateResponseTest.php

@@ -5,6 +5,7 @@
 namespace Firehed\WebAuthn;
 
 use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
 #[CoversClass(CreateResponse::class)]
@@ -406,12 +407,70 @@ public function testTransportsEndUpInCredential(): void
         ], $cred->getTransports());
     }
 
+    // 7.1.7, 7.1.8, 7.1.9
+    #[DataProvider('requiredClientDataMembers')]
+    public function testAbsentCDJMemberIsError(string $member): void
+    {
+        $cdj = $this->decodedClientData();
+        unset($cdj[$member]);
+
+        $this->expectRegistrationError('7.1.x');
+        $this->responseWithClientData($cdj)->verify($this->cm, $this->rp);
+    }
+
+    // 7.1.7, 7.1.8, 7.1.9
+    #[DataProvider('requiredClientDataMembers')]
+    public function testNonStringCDJMemberIsError(string $member): void
+    {
+        $cdj = $this->decodedClientData();
+        $cdj[$member] = 42;
+
+        $this->expectRegistrationError('7.1.x');
+        $this->responseWithClientData($cdj)->verify($this->cm, $this->rp);
+    }
+
+    /**
+     * @return array<string, array{string}>
+     */
+    public static function requiredClientDataMembers(): array
+    {
+        return [
+            'type' => ['type'],
+            'challenge' => ['challenge'],
+            'origin' => ['origin'],
+        ];
+    }
+
     private function expectRegistrationError(string $section): void
     {
         $this->expectException(Errors\RegistrationError::class);
         // TODO: how to assert on $section
     }
 
+    /**
+     * @return mixed[]
+     */
+    private function decodedClientData(): array
+    {
+        $cdj = json_decode($this->clientDataJson->unwrap(), true, flags: JSON_THROW_ON_ERROR);
+        assert(is_array($cdj));
+        return $cdj;
+    }
+
+    /**
+     * @param mixed[] $cdj
+     */
+    private function responseWithClientData(array $cdj): CreateResponse
+    {
+        return new CreateResponse(
+            type: Enums\PublicKeyCredentialType::PublicKey,
+            id: $this->id,
+            ao: $this->attestationObject,
+            clientDataJson: new BinaryString(json_encode($cdj, JSON_THROW_ON_ERROR)),
+            transports: [],
+        );
+    }
+
     private function getDefaultResponse(): CreateResponse
     {
         return new CreateResponse(

tests/GetResponseTest.php

@@ -5,6 +5,7 @@
 namespace Firehed\WebAuthn;
 
 use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
 #[CoversClass(GetResponse::class)]
@@ -354,9 +355,67 @@ public function testUserHandleWithValue(): void
         self::assertSame($handle, $response->getUserHandle());
     }
 
+    // 7.2.11, 7.2.12, 7.2.13
+    #[DataProvider('requiredClientDataMembers')]
+    public function testAbsentCDJMemberIsError(string $member): void
+    {
+        $cdj = $this->decodedClientData();
+        unset($cdj[$member]);
+
+        $this->expectVerificationError('7.2.x');
+        $this->responseWithClientData($cdj)->verify($this->cm, $this->rp, $this->credential);
+    }
+
+    // 7.2.11, 7.2.12, 7.2.13
+    #[DataProvider('requiredClientDataMembers')]
+    public function testNonStringCDJMemberIsError(string $member): void
+    {
+        $cdj = $this->decodedClientData();
+        $cdj[$member] = 42;
+
+        $this->expectVerificationError('7.2.x');
+        $this->responseWithClientData($cdj)->verify($this->cm, $this->rp, $this->credential);
+    }
+
+    /**
+     * @return array<string, array{string}>
+     */
+    public static function requiredClientDataMembers(): array
+    {
+        return [
+            'type' => ['type'],
+            'challenge' => ['challenge'],
+            'origin' => ['origin'],
+        ];
+    }
+
     private function expectVerificationError(string $section): void
     {
         $this->expectException(Errors\VerificationError::class);
         // TODO: how to assert on $section
     }
+
+    /**
+     * @return mixed[]
+     */
+    private function decodedClientData(): array
+    {
+        $cdj = json_decode($this->clientDataJson->unwrap(), true, flags: JSON_THROW_ON_ERROR);
+        assert(is_array($cdj));
+        return $cdj;
+    }
+
+    /**
+     * @param mixed[] $cdj
+     */
+    private function responseWithClientData(array $cdj): GetResponse
+    {
+        return new GetResponse(
+            credentialId: $this->id,
+            rawAuthenticatorData: $this->rawAuthenticatorData,
+            clientDataJson: new BinaryString(json_encode($cdj, JSON_THROW_ON_ERROR)),
+            signature: $this->signature,
+            userHandle: null,
+        );
+    }
 }