From 95dcc610fcadb3690bed996fd11cf8d386a103b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 20 Sep 2022 12:14:33 +0200 Subject: [PATCH 1/7] Harden tests for local IP detection in URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/Http/Client/ClientTest.php | 1 + .../Http/Client/LocalAddressCheckerTest.php | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/tests/lib/Http/Client/ClientTest.php b/tests/lib/Http/Client/ClientTest.php index 141c6190cd97d..25d4749df574c 100644 --- a/tests/lib/Http/Client/ClientTest.php +++ b/tests/lib/Http/Client/ClientTest.php @@ -161,6 +161,7 @@ public function dataPreventLocalAddress():array { ['another-host.local'], ['service.localhost'], ['!@#$'], // test invalid url + ['normal.host.com'], ]; } diff --git a/tests/lib/Http/Client/LocalAddressCheckerTest.php b/tests/lib/Http/Client/LocalAddressCheckerTest.php index 9f2f6c72993e3..991801f043dd2 100644 --- a/tests/lib/Http/Client/LocalAddressCheckerTest.php +++ b/tests/lib/Http/Client/LocalAddressCheckerTest.php @@ -121,6 +121,25 @@ public function dataPreventLocalAddress():array { ['100.100.100.200'], ['192.0.0.1'], ['randomdomain.internal'], + ['0177.0.0.9'], + ['⑯⑨。②⑤④。⑯⑨｡②⑤④'], + ['127。②⑤④。⑯⑨.②⑤④'], + ['127.0.00000000000000000000000000000000001'], + ['127.1'], + ['127.000.001'], + ['0177.0.0.01'], + ['0x7f.0x0.0x0.0x1'], + ['0x7f000001'], + ['2130706433'], + ['00000000000000000000000000000000000000000000000000177.1'], + ['0x7f.1'], + ['127.0x1'], + ['[0000:0000:0000:0000:0000:0000:0000:0001]'], + ['[0:0:0:0:0:0:0:1]'], + ['[0:0:0:0::0:0:1]'], + ['%31%32%37%2E%30%2E%30%2E%31'], + ['%31%32%37%2E%30%2E%30.%31'], + ['[%3A%3A%31]'], ]; } From b2a893abad3fcf4b27806613080b9a5df29572b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 20 Sep 2022 12:20:35 +0200 Subject: [PATCH 2/7] Add missing urldecode and idn_to_utf8 calls to local address checker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The call to idn_to_utf8 call is actually to apply normalization Signed-off-by: Côme Chilliet --- lib/private/Http/Client/DnsPinMiddleware.php | 2 +- lib/private/Http/Client/LocalAddressChecker.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/Http/Client/DnsPinMiddleware.php b/lib/private/Http/Client/DnsPinMiddleware.php index ee0ea053dcfe6..f5e6214a4ab45 100644 --- a/lib/private/Http/Client/DnsPinMiddleware.php +++ b/lib/private/Http/Client/DnsPinMiddleware.php @@ -125,7 +125,7 @@ public function addDnsPinning() { $ports[] = (string)$port; } - $targetIps = $this->dnsResolve($hostName, 0); + $targetIps = $this->dnsResolve(idn_to_utf8($hostName), 0); $curlResolves = []; diff --git a/lib/private/Http/Client/LocalAddressChecker.php b/lib/private/Http/Client/LocalAddressChecker.php index f4fea503ab9d5..b0de54b430fa0 100644 --- a/lib/private/Http/Client/LocalAddressChecker.php +++ b/lib/private/Http/Client/LocalAddressChecker.php @@ -72,7 +72,7 @@ public function ThrowIfLocalAddress(string $uri) : void { throw new LocalServerException('Could not detect any host'); } - $host = strtolower($host); + $host = idn_to_utf8(strtolower(urldecode($host))); // Remove brackets from IPv6 addresses if (strpos($host, '[') === 0 && substr($host, -1) === ']') { $host = substr($host, 1, -1); From 060230eec7b962a2854bd2a79ffae668049d437c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 20 Sep 2022 12:23:24 +0200 Subject: [PATCH 3/7] Add mlocati/ip-lib dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- 3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty b/3rdparty index f143482ffb0b8..27cc868e57475 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit f143482ffb0b8dfdbc08cd848ce2e66f02a5d9b6 +Subproject commit 27cc868e57475a71b92cc037c869205765d36675 From 515e05cf16f7d2407388d0a3120d019820f08fe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 20 Sep 2022 12:34:04 +0200 Subject: [PATCH 4/7] Use new dependency to normalize IPs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Http/Client/LocalAddressChecker.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/private/Http/Client/LocalAddressChecker.php b/lib/private/Http/Client/LocalAddressChecker.php index b0de54b430fa0..274186d0a5a93 100644 --- a/lib/private/Http/Client/LocalAddressChecker.php +++ b/lib/private/Http/Client/LocalAddressChecker.php @@ -25,6 +25,8 @@ */ namespace OC\Http\Client; +use IPLib\Factory; +use IPLib\ParseStringFlag; use OCP\Http\Client\LocalServerException; use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\IpUtils; @@ -37,6 +39,17 @@ public function __construct(LoggerInterface $logger) { } public function ThrowIfLocalIp(string $ip) : void { + $parsedIp = Factory::parseAddressString( + $ip, + ParseStringFlag::IPV4_MAYBE_NON_DECIMAL | ParseStringFlag::IPV4ADDRESS_MAYBE_NON_QUAD_DOTTED + ); + if ($parsedIp === null) { + /* Not an IP */ + return; + } + /* Replace by normalized form */ + $ip = (string)$parsedIp; + $localRanges = [ '100.64.0.0/10', // See RFC 6598 '192.0.0.0/24', // See RFC 6890 From 3c47caf08b4eab55264c5a1814e31805fc49a819 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 20 Sep 2022 12:46:22 +0200 Subject: [PATCH 5/7] Fix tests for nested v4 in v6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../Http/Client/LocalAddressChecker.php | 20 ++++++------------- .../Http/Client/LocalAddressCheckerTest.php | 4 ++-- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/lib/private/Http/Client/LocalAddressChecker.php b/lib/private/Http/Client/LocalAddressChecker.php index 274186d0a5a93..13a7d062de3bd 100644 --- a/lib/private/Http/Client/LocalAddressChecker.php +++ b/lib/private/Http/Client/LocalAddressChecker.php @@ -25,6 +25,7 @@ */ namespace OC\Http\Client; +use IPLib\Address\IPv6; use IPLib\Factory; use IPLib\ParseStringFlag; use OCP\Http\Client\LocalServerException; @@ -48,7 +49,11 @@ public function ThrowIfLocalIp(string $ip) : void { return; } /* Replace by normalized form */ - $ip = (string)$parsedIp; + if ($parsedIp instanceof IPv6) { + $ip = (string)($parsedIp->toIPv4() ?? $parsedIp); + } else { + $ip = (string)$parsedIp; + } $localRanges = [ '100.64.0.0/10', // See RFC 6598 @@ -63,19 +68,6 @@ public function ThrowIfLocalIp(string $ip) : void { $this->logger->warning("Host $ip was not connected to because it violates local access rules"); throw new LocalServerException('Host violates local access rules'); } - - // Also check for IPv6 IPv4 nesting, because that's not covered by filter_var - if ((bool)filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) && substr_count($ip, '.') > 0) { - $delimiter = strrpos($ip, ':'); // Get last colon - $ipv4Address = substr($ip, $delimiter + 1); - - if ( - !filter_var($ipv4Address, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) || - IpUtils::checkIp($ip, $localRanges)) { - $this->logger->warning("Host $ip was not connected to because it violates local access rules"); - throw new LocalServerException('Host violates local access rules'); - } - } } public function ThrowIfLocalAddress(string $uri) : void { diff --git a/tests/lib/Http/Client/LocalAddressCheckerTest.php b/tests/lib/Http/Client/LocalAddressCheckerTest.php index 991801f043dd2..8c8e64eddf928 100644 --- a/tests/lib/Http/Client/LocalAddressCheckerTest.php +++ b/tests/lib/Http/Client/LocalAddressCheckerTest.php @@ -91,7 +91,7 @@ public function dataInternalIPs() : array { return [ ['192.168.0.1'], ['fe80::200:5aee:feaa:20a2'], - ['0:0:0:0:0:0:10.0.0.1'], + ['0:0:0:0:0:ffff:10.0.0.1'], ['0:0:0:0:0:ffff:127.0.0.0'], ['10.0.0.1'], ['::'], @@ -112,7 +112,7 @@ public function dataPreventLocalAddress():array { ['172.16.42.1'], ['[fdf8:f53b:82e4::53]/secret.ics'], ['[fe80::200:5aee:feaa:20a2]/secret.ics'], - ['[0:0:0:0:0:0:10.0.0.1]/secret.ics'], + ['[0:0:0:0:0:ffff:10.0.0.1]/secret.ics'], ['[0:0:0:0:0:ffff:127.0.0.0]/secret.ics'], ['10.0.0.1'], ['another-host.local'], From 2948697257168337eee074ca7773717a14c2b621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 20 Sep 2022 17:49:17 +0200 Subject: [PATCH 6/7] Update 3rdparty to master MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- 3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty b/3rdparty index 27cc868e57475..3095d4062823f 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit 27cc868e57475a71b92cc037c869205765d36675 +Subproject commit 3095d4062823f3f913d594f9ff313010ed55cd74 From 205760a3aa79c8aa9cc8ca6a70a8bf35cf2292a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 22 Sep 2022 10:16:35 +0200 Subject: [PATCH 7/7] Fix idn_to_utf8 stub signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- build/stubs/intl.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/stubs/intl.php b/build/stubs/intl.php index 201db9a33f4e1..08e0c71933809 100644 --- a/build/stubs/intl.php +++ b/build/stubs/intl.php @@ -4622,7 +4622,7 @@ function idn_to_ascii($domain, $options = 0, $variant = INTL_IDNA_VARIANT_2003, * @param int $variant [optional]

* Either INTL_IDNA_VARIANT_2003 for IDNA 2003 or INTL_IDNA_VARIANT_UTS46 for UTS #46. *

- * @param int &$idna_info [optional]

+ * @param array &$idna_info [optional]

* This parameter can be used only if INTL_IDNA_VARIANT_UTS46 was used for variant. * In that case, it will be filled with an array with the keys 'result', * the possibly illegal result of the transformation, 'isTransitionalDifferent', @@ -4634,7 +4634,7 @@ function idn_to_ascii($domain, $options = 0, $variant = INTL_IDNA_VARIANT_2003, * RFC 3490 4.2 states though "ToUnicode never fails. If any step fails, then the original input * sequence is returned immediately in that step." */ -function idn_to_utf8($domain, $options = 0, $variant = INTL_IDNA_VARIANT_2003, array &$idna_info) { } +function idn_to_utf8($domain, $options = 0, $variant = INTL_IDNA_VARIANT_2003, array &$idna_info = null) { } /** * (PHP 5 >=5.5.0 PECL intl >= 3.0.0a1)