Skip to content

Commit 3b5f5ef

Browse files
SpomkySpomky
authored
Merge pull request #813 from web-auth/merge-up/5.2.x-to-5.3.x
fix: merge up CVE fix (GHSA-f7pm-6hr8-7ggm) from 5.2.x
2 parents 65df13a + 90df1d3 commit 3b5f5ef

File tree

5 files changed

+117
-32
lines changed

5 files changed

+117
-32
lines changed

‎src/webauthn/src/CeremonyStep/CheckAllowedOrigins.php‎

Lines changed: 112 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,18 @@
2323
final readonly class CheckAllowedOrigins implements CeremonyStep
2424
{
2525
/**
26+
* Full origin entries (scheme://host[:port]) from allowed origins that include a scheme.
27+
*
2628
* @var string[]
2729
*/
28-
private array $allowedOrigins;
30+
private array $fullOrigins;
31+
32+
/**
33+
* Host-only entries from allowed origins without a scheme (backward compatibility).
34+
*
35+
* @var string[]
36+
*/
37+
private array $hostOrigins;
2938

3039
/**
3140
* @param string[] $allowedOrigins
@@ -34,15 +43,20 @@ public function __construct(
3443
array $allowedOrigins,
3544
private bool $allowSubdomains = false
3645
) {
46+
$fullOrigins = [];
47+
$hostOrigins = [];
3748
foreach ($allowedOrigins as $allowedOrigin) {
38-
$parsedAllowedOrigin = parse_url($allowedOrigin);
39-
$parsedAllowedOrigin !== false || throw new InvalidArgumentException(sprintf(
40-
'Invalid origin: %s',
41-
$allowedOrigin
42-
));
49+
$parsed = parse_url($allowedOrigin);
50+
$parsed !== false || throw new InvalidArgumentException(sprintf('Invalid origin: %s', $allowedOrigin));
51+
if (isset($parsed['scheme'], $parsed['host'])) {
52+
$fullOrigins[] = self::buildOrigin($parsed['scheme'], $parsed['host'], $parsed['port'] ?? null);
53+
} else {
54+
$hostOrigins[] = $parsed['host'] ?? $allowedOrigin;
55+
}
4356
}
4457

45-
$this->allowedOrigins = array_unique($allowedOrigins);
58+
$this->fullOrigins = array_unique($fullOrigins);
59+
$this->hostOrigins = array_unique($hostOrigins);
4660
}
4761

4862
public function process(
@@ -63,29 +77,43 @@ public function process(
6377
$authData = $authenticatorResponse instanceof AuthenticatorAssertionResponse ? $authenticatorResponse->authenticatorData : $authenticatorResponse->attestationObject->authData;
6478
$C = $authenticatorResponse->clientDataJSON;
6579

66-
$parsedRelyingPartyId = parse_url($C->origin);
67-
$clientDataRpId = $parsedRelyingPartyId['host'] ?? '';
68-
if ($clientDataRpId === '') {
69-
$clientDataRpId = $C->origin;
70-
}
71-
is_array($parsedRelyingPartyId) || throw AuthenticatorResponseVerificationException::create(
80+
$parsedOrigin = parse_url($C->origin);
81+
is_array($parsedOrigin) || throw AuthenticatorResponseVerificationException::create(
7282
'Invalid origin. Unable to parse the origin.'
7383
);
74-
if (in_array($C->origin, $this->allowedOrigins, true)) {
75-
return;
76-
}
77-
$allowedHosts = array_map(
78-
static fn (string $origin): string => parse_url($origin, PHP_URL_HOST) ?? $origin,
79-
$this->allowedOrigins
80-
);
81-
$isSubDomain = $this->isSubdomain($allowedHosts, $clientDataRpId);
82-
if ($this->allowSubdomains && $isSubDomain) {
83-
return;
84-
}
85-
if (! $this->allowSubdomains && $isSubDomain) {
86-
throw AuthenticatorResponseVerificationException::create('Invalid origin. Subdomains are not allowed.');
87-
}
88-
if (count($this->allowedOrigins) !== 0) {
84+
$originHost = $parsedOrigin['host'] ?? $C->origin;
85+
86+
$hasAllowedOrigins = count($this->fullOrigins) !== 0 || count($this->hostOrigins) !== 0;
87+
88+
if ($hasAllowedOrigins) {
89+
// Full origin match (scheme + host + port)
90+
if (isset($parsedOrigin['scheme'], $parsedOrigin['host'])) {
91+
$normalizedOrigin = self::buildOrigin(
92+
$parsedOrigin['scheme'],
93+
$parsedOrigin['host'],
94+
$parsedOrigin['port'] ?? null
95+
);
96+
if (in_array($normalizedOrigin, $this->fullOrigins, true)) {
97+
return;
98+
}
99+
}
100+
101+
// Host-only match (backward compatibility for entries without scheme)
102+
if (in_array($originHost, $this->hostOrigins, true)) {
103+
return;
104+
}
105+
106+
// Subdomain matching
107+
$isFullOriginSubdomain = $this->isSubdomainOfFullOrigins($parsedOrigin);
108+
$isHostSubdomain = $this->isSubdomain($this->hostOrigins, $originHost);
109+
$isSubDomain = $isFullOriginSubdomain || $isHostSubdomain;
110+
111+
if ($this->allowSubdomains && $isSubDomain) {
112+
return;
113+
}
114+
if (! $this->allowSubdomains && $isSubDomain) {
115+
throw AuthenticatorResponseVerificationException::create('Invalid origin. Subdomains are not allowed.');
116+
}
89117
throw AuthenticatorResponseVerificationException::create(
90118
'Invalid origin. Not in the list of allowed origins.'
91119
);
@@ -96,23 +124,76 @@ public function process(
96124
$facetId !== '' || throw AuthenticatorResponseVerificationException::create(
97125
'Invalid origin. Unable to determine the facet ID.'
98126
);
99-
if ($clientDataRpId === $facetId) {
127+
if ($originHost === $facetId) {
100128
return;
101129
}
102-
$isSubDomains = $this->isSubdomainOf($clientDataRpId, $facetId);
130+
$isSubDomains = $this->isSubdomainOf($originHost, $facetId);
103131
if ($this->allowSubdomains && $isSubDomains) {
104132
return;
105133
}
106134
if (! $this->allowSubdomains && $isSubDomains) {
107135
throw AuthenticatorResponseVerificationException::create('Invalid origin. Subdomains are not allowed.');
108136
}
109137

110-
$scheme = $parsedRelyingPartyId['scheme'] ?? '';
138+
$scheme = $parsedOrigin['scheme'] ?? '';
111139
$scheme === 'https' || throw AuthenticatorResponseVerificationException::create(
112140
'Invalid scheme. HTTPS required.'
113141
);
114142
}
115143

144+
/**
145+
* @param array<string, mixed> $parsedOrigin Parsed origin from parse_url()
146+
*/
147+
private function isSubdomainOfFullOrigins(array $parsedOrigin): bool
148+
{
149+
if (! isset($parsedOrigin['scheme'], $parsedOrigin['host'])) {
150+
return false;
151+
}
152+
/** @var string $originScheme */
153+
$originScheme = $parsedOrigin['scheme'];
154+
/** @var string $originHost */
155+
$originHost = $parsedOrigin['host'];
156+
$originPort = $parsedOrigin['port'] ?? null;
157+
158+
foreach ($this->fullOrigins as $fullOrigin) {
159+
$parsedAllowed = parse_url($fullOrigin);
160+
if (! is_array($parsedAllowed) || ! isset($parsedAllowed['scheme'], $parsedAllowed['host'])) {
161+
continue;
162+
}
163+
/** @var string $allowedScheme */
164+
$allowedScheme = $parsedAllowed['scheme'];
165+
/** @var string $allowedHost */
166+
$allowedHost = $parsedAllowed['host'];
167+
if ($originScheme !== $allowedScheme) {
168+
continue;
169+
}
170+
$allowedPort = $parsedAllowed['port'] ?? null;
171+
if ($originPort !== $allowedPort) {
172+
continue;
173+
}
174+
if ($this->isSubdomainOf($originHost, $allowedHost)) {
175+
return true;
176+
}
177+
}
178+
179+
return false;
180+
}
181+
182+
private static function buildOrigin(string $scheme, string $host, ?int $port): string
183+
{
184+
if ($port === null) {
185+
return sprintf('%s://%s', $scheme, $host);
186+
}
187+
$defaultPorts = [
188+
'https' => 443,
189+
'http' => 80,
190+
];
191+
if (isset($defaultPorts[$scheme]) && $port === $defaultPorts[$scheme]) {
192+
return sprintf('%s://%s', $scheme, $host);
193+
}
194+
return sprintf('%s://%s:%d', $scheme, $host, $port);
195+
}
196+
116197
private function isSubdomainOf(string $subdomain, string $domain): bool
117198
{
118199
return str_ends_with('.' . $subdomain, '.' . $domain);

‎tests/library/AbstractTestCase.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ protected function getCeremonyStepManagerFactory(): CeremonyStepManagerFactory
105105
$this->ceremonyStepManagerFactory->setAllowedOrigins([
106106
'http://localhost',
107107
'https://localhost',
108+
'https://localhost:8443',
108109
'https://dev.dontneeda.pw',
109110
'https://spomky-webauthn.herokuapp.com',
110111
'https://tuleap-web.tuleap-aio-dev.docker',

‎tests/library/Functional/CheckAllowedOriginsTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
namespace Webauthn\Tests\Functional;
66

7+
use const JSON_THROW_ON_ERROR;
8+
use const JSON_UNESCAPED_SLASHES;
79
use PHPUnit\Framework\Attributes\Test;
810
use Symfony\Component\Uid\Uuid;
911
use Webauthn\CeremonyStep\CheckAllowedOrigins;

‎tests/symfony/config/config.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ webauthn:
122122
options_storage: 'Webauthn\Tests\Bundle\Functional\CustomSessionStorage'
123123
allowed_origins:
124124
- 'https://localhost'
125+
- 'https://localhost:8443'
125126
- 'https://bar.acme'
126127
- 'https://webauthn.spomky-labs.com'
127128
- 'https://spomky-webauthn.herokuapp.com'

‎tests/symfony/functional/Firewall/AllowedOriginsTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public function allowedOriginsAreAvailable(): void
2727
//Then
2828
static::assertResponseIsSuccessful();
2929
static::assertSame(
30-
'{"origins":["https:\/\/localhost","https:\/\/bar.acme","https:\/\/webauthn.spomky-labs.com","https:\/\/spomky-webauthn.herokuapp.com"]}',
30+
'{"origins":["https:\/\/localhost","https:\/\/localhost:8443","https:\/\/bar.acme","https:\/\/webauthn.spomky-labs.com","https:\/\/spomky-webauthn.herokuapp.com"]}',
3131
$client->getResponse()
3232
->getContent()
3333
);

0 commit comments

Comments
 (0)