From 6760c2aaa2711aaa61e2f5cb7e8a36e8d190b2e6 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Tue, 11 Aug 2026 11:56:05 -0700 Subject: [PATCH 1/2] Guard clientDataJSON members in registration --- src/CreateResponse.php | 13 +++++--- tests/CreateResponseTest.php | 59 ++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/CreateResponse.php b/src/CreateResponse.php index c2b9d1a..d38ce33 100644 --- a/src/CreateResponse.php +++ b/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'); } diff --git a/tests/CreateResponseTest.php b/tests/CreateResponseTest.php index 6acb0ea..5404e99 100644 --- a/tests/CreateResponseTest.php +++ b/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 + */ + 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( From 5e15d87240222a54499af1177ca95151e310214a Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Tue, 11 Aug 2026 11:57:44 -0700 Subject: [PATCH 2/2] Guard clientDataJSON members in authentication --- src/GetResponse.php | 14 ++++++---- tests/GetResponseTest.php | 59 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/src/GetResponse.php b/src/GetResponse.php index 4e615b7..acf8db4 100644 --- a/src/GetResponse.php +++ b/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'); } diff --git a/tests/GetResponseTest.php b/tests/GetResponseTest.php index 39fe05e..3c4b5c4 100644 --- a/tests/GetResponseTest.php +++ b/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 + */ + 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, + ); + } }