Skip to content

Commit a4abf48

Browse files
bnfohader
bnf
authored and
ohader
committed
[SECURITY] Circumvent parser deviation in PSR-7 URI object
Adapt the URI object to refuse construction based on invalid URL inputs. As `parse_url()` is defined to not validate the resulting parts of the parsed URL, a parser deviation between PHP and browser URL implementations is possible when URL parts contain invalid characters. The missing validation is now performed via PHP upstream rules built into `filter_var()`. Note that `filter_var` has two limitations: It requires a fully qualified URL and it can not validate IDN domain names that have not been converted to ascii before. In order to circumvent these limitations we'll imply a intermediate fake default-scheme and perform IDN-to-ascii conversion for the purpose of the validation. Resolves: #105170 Releases: main, 13.4, 12.4 Change-Id: I63d02de49de53513a8f3b4442a35dccb01b92eff Security-Bulletin: TYPO3-CORE-SA-2025-002 Security-References: CVE-2024-55892 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87743 Reviewed-by: Oliver Hader <oliver.hader@typo3.org> Tested-by: Oliver Hader <oliver.hader@typo3.org>
1 parent 656a33d commit a4abf48

File tree

3 files changed

+171
-6
lines changed

3 files changed

+171
-6
lines changed

‎typo3/sysext/core/Classes/Http/Uri.php

+43-6
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public static function fromAnyScheme(string $uri = ''): self
117117

118118
/**
119119
* @param string $uri The full URI including query string and fragment
120-
* @throws \InvalidArgumentException when the URI is not a string
120+
* @throws \InvalidArgumentException if the URI is malformed.
121121
*/
122122
public function __construct(string $uri = '')
123123
{
@@ -154,7 +154,14 @@ protected function parseUri(string $uri): void
154154
}
155155
}
156156
if (isset($uriParts['port'])) {
157-
$this->port = (int)$uriParts['port'];
157+
$port = (int)$uriParts['port'];
158+
if (!$this->validatePort($port)) {
159+
throw new \InvalidArgumentException(
160+
'The uri "' . $uri . '" appears to be malformed, invalid port "' . $port . '" specified, must be a valid TCP/UDP port',
161+
1728057215
162+
);
163+
}
164+
$this->port = $port;
158165
}
159166
if (isset($uriParts['path'])) {
160167
$this->path = $this->sanitizePath($uriParts['path']);
@@ -165,6 +172,30 @@ protected function parseUri(string $uri): void
165172
if (isset($uriParts['fragment'])) {
166173
$this->fragment = $this->sanitizeFragment($uriParts['fragment']);
167174
}
175+
176+
if (!$this->validate()) {
177+
throw new \InvalidArgumentException('The uri "' . $uri . '" appears to be malformed', 1728057216);
178+
}
179+
}
180+
181+
protected function validate(): bool
182+
{
183+
$url = clone $this;
184+
185+
if ($url->scheme === '') {
186+
// filter_var will mark //example.com/ as invalid, let's pretend it's https in this case
187+
$url->scheme = 'https';
188+
}
189+
190+
if ($url->host === '') {
191+
// filter_var will mark /mypath/ as invalid, let's pretend it's localhost in this case
192+
$url->host = 'localhost';
193+
} else {
194+
// filter_var can not validate UTF8 encoded hosts
195+
$url->host = idn_to_ascii($url->host);
196+
}
197+
198+
return filter_var($url->__toString(), FILTER_VALIDATE_URL) !== false;
168199
}
169200

170201
/**
@@ -445,17 +476,23 @@ public function withHost(string $host): UriInterface
445476
*/
446477
public function withPort(?int $port): UriInterface
447478
{
448-
if ($port !== null) {
449-
if ($port < 1 || $port > 65535) {
450-
throw new \InvalidArgumentException('Invalid port "' . $port . '" specified, must be a valid TCP/UDP port.', 1436717326);
451-
}
479+
if ($port !== null && !$this->validatePort($port)) {
480+
throw new \InvalidArgumentException('Invalid port "' . $port . '" specified, must be a valid TCP/UDP port.', 1436717326);
452481
}
453482

454483
$clonedObject = clone $this;
455484
$clonedObject->port = $port;
456485
return $clonedObject;
457486
}
458487

488+
protected function validatePort(int $port): bool
489+
{
490+
if ($port < 1 || $port > 65535) {
491+
return false;
492+
}
493+
return true;
494+
}
495+
459496
/**
460497
* Return an instance with the specified path.
461498
*

‎typo3/sysext/core/Classes/Security/ContentSecurityPolicy/UriValue.php

+14
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,20 @@ public static function fromUri(UriInterface $other): self
3636
return new self((string)$other);
3737
}
3838

39+
protected function validate(): bool
40+
{
41+
$backupHost = null;
42+
if ($this->host) {
43+
$backupHost = $this->host;
44+
$this->host = str_replace('*', 'wildcard', $this->host);
45+
}
46+
$ret = parent::validate();
47+
if ($backupHost !== null) {
48+
$this->host = $backupHost;
49+
}
50+
return $ret;
51+
}
52+
3953
public function __toString(): string
4054
{
4155
if ($this->entireWildcard) {

‎typo3/sysext/core/Tests/Unit/Http/UriTest.php

+114
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,33 @@ public function withPortAndNullValueReturnsInstanceWithProvidedPort(): void
110110
public function noSchemeWithDomainAlikeIsInterpretedAsPath(): void
111111
{
112112
$subject = new Uri('www.example.com');
113+
// This is counter intuitive, but interpreted as path.
114+
// Although we'd like to see protocol independent `//` here,
115+
// we must not change this, as…
113116
self::assertEquals('/www.example.com', (string)$subject);
117+
118+
// …this behaviour is security relevant.
119+
// This invalid domain name – given without a scheme – is
120+
// only "save" because they are considered to be paths
121+
// (invalid hostname alike is not parsed as a hostname):
122+
$subject = new Uri('evil.tld\\@host.tld');
123+
self::assertEquals('/evil.tld%5C@host.tld', (string)$subject);
124+
125+
$subject = new Uri('evil.tld\\\\\\@host.tld');
126+
self::assertEquals('/evil.tld%5C%5C%5C@host.tld', (string)$subject);
114127
}
115128

116129
#[Test]
117130
public function noSchemeWithDomainAlikeAndTrailingSlashIsInterpretedAsPath(): void
118131
{
119132
$subject = new Uri('www.example.com/');
120133
self::assertEquals('/www.example.com/', (string)$subject);
134+
135+
$subject = new Uri('evil.tld\\@host.tld/');
136+
self::assertEquals('/evil.tld%5C@host.tld/', (string)$subject);
137+
138+
$subject = new Uri('evil.tld\\\\\\@host.tld/');
139+
self::assertEquals('/evil.tld%5C%5C%5C@host.tld/', (string)$subject);
121140
}
122141

123142
public static function validPortsDataProvider(): array
@@ -497,6 +516,69 @@ public function canParseInternationalizedDomainName(): void
497516
self::assertEquals('https://ουτοπία.δπθ.gr/', (string)$uri);
498517
}
499518

519+
public static function invalidHostDataProvider(): \Generator
520+
{
521+
yield [
522+
'url' => 'https://evil.tld\\@host.tld/',
523+
'parseUrl' => 'host.tld',
524+
'chrome' => 'evil.tld',
525+
];
526+
527+
yield [
528+
'url' => 'https://evil.tld\\\\@host.tld/',
529+
'parseUrl' => 'host.tld',
530+
'chrome' => 'evil.tld',
531+
];
532+
533+
yield [
534+
'url' => 'https://evil.tld\\\\\\@host.tld/',
535+
'parseUrl' => 'host.tld',
536+
'chrome' => 'evil.tld',
537+
];
538+
539+
yield [
540+
'url' => '//evil.tld\\@host.tld/',
541+
'parseUrl' => 'host.tld',
542+
'chrome' => 'evil.tld',
543+
];
544+
545+
yield [
546+
'url' => '//evil.tld\\\\\\@host.tld',
547+
'parseUrl' => 'host.tld',
548+
'chrome' => 'evil.tld',
549+
];
550+
551+
yield [
552+
'url' => 'http://normal.com[@evil.tld]/',
553+
'parseUrl' => 'evil.tld]',
554+
'chrome' => 'evil.tld',
555+
];
556+
557+
yield [
558+
'url' => 'http://evil.tld\\',
559+
'parseUrl' => 'evil.tld\\',
560+
'chrome' => 'evil.tld',
561+
];
562+
}
563+
564+
#[DataProvider('invalidHostDataProvider')]
565+
#[Test]
566+
public function uriParsingThrowsExceptionForParserDeviatingHostValues($url, $parseUrl, $chrome): void
567+
{
568+
$this->expectException(\InvalidArgumentException::class);
569+
$this->expectExceptionCode(1728057216);
570+
new Uri($url);
571+
}
572+
573+
#[DataProvider('invalidHostDataProvider')]
574+
#[Test]
575+
public function parseUrlReturnsExpectedInvalidHostForInvalidHostURLs($url, $parseUrl, $chrome): void
576+
{
577+
$result = parse_url($url, PHP_URL_HOST);
578+
self::assertSame($parseUrl, $result);
579+
self::assertNotSame($chrome, $result);
580+
}
581+
500582
#[Test]
501583
public function canParseAllPropertiesWithIPv6(): void
502584
{
@@ -563,4 +645,36 @@ public function canStringifyMalformedBracketlessIPv6toValidIPv6(): void
563645
$uri = new Uri($input);
564646
self::assertSame($expected, (string)$uri);
565647
}
648+
649+
/**
650+
* @return iterable<non-empty-string, array{non-empty-string}>
651+
*/
652+
public static function invalidUriProvider(): iterable
653+
{
654+
yield 'Unsupported scheme ftp' => ['ftp://user:pass@local.example.com:3001/foo?bar=baz#quz'];
655+
yield 'Unsupported scheme ssh' => ['ssh://user:pass@local.example.com:3001/foo?bar=baz#quz'];
656+
yield 'Unsupported scheme git' => ['git://user:pass@local.example.com:3001/foo?bar=baz#quz'];
657+
658+
yield 'Invalid port -1 (too small)' => ['https://user:pass@local.example.com:-1/foo?bar=baz#quz'];
659+
yield 'Invalid port 0 (zero)' => ['https://user:pass@local.example.com:0/foo?bar=baz#quz'];
660+
yield 'Invalid port 65536 (too big)' => ['https://user:pass@local.example.com:65536/foo?bar=baz#quz'];
661+
662+
yield from [
663+
'Malformed URI' => ['http://invalid:%20https://example.com'],
664+
'Colon in non-IPv6 host' => ['https://user:pass@local:example.com:3001/foo?bar=baz#quz'],
665+
'Wrong bracket in the IPv6' => ['https://user:pass@fe80[::200:5aee:feaa:20a2]:3001/foo?bar=baz#quz'],
666+
// percent encoding is allowed in URI but not in web urls particularly with idn encoding for dns.
667+
// no validation for correct percent encoding either
668+
// 'Percent in the host' => ["https://user:pass@local%example.com:3001/foo?bar=baz#quz"],
669+
'Bracket in the host' => ['https://user:pass@[local.example.com]:3001/foo?bar=baz#quz'],
670+
];
671+
}
672+
673+
#[DataProvider('invalidUriProvider')]
674+
#[Test]
675+
public function invalidUriRaisesAnException(string $invalidUri): void
676+
{
677+
$this->expectException(\InvalidArgumentException::class);
678+
new Uri($invalidUri);
679+
}
566680
}

0 commit comments

Comments
 (0)