From c083f77e4e88c14ea2515440aae40234ee35ce0a Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Tue, 20 Sep 2022 12:14:33 +0200 Subject: 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 + tests/lib/Http/Client/LocalAddressCheckerTest.php | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) (limited to 'tests/lib') diff --git a/tests/lib/Http/Client/ClientTest.php b/tests/lib/Http/Client/ClientTest.php index 141c6190cd9..25d4749df57 100644 --- a/tests/lib/Http/Client/ClientTest.php +++ b/tests/lib/Http/Client/ClientTest.php @@ -161,6 +161,7 @@ class ClientTest extends \Test\TestCase { ['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 9f2f6c72993..991801f043d 100644 --- a/tests/lib/Http/Client/LocalAddressCheckerTest.php +++ b/tests/lib/Http/Client/LocalAddressCheckerTest.php @@ -121,6 +121,25 @@ class LocalAddressCheckerTest extends \Test\TestCase { ['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]'], ]; } -- cgit v1.2.3 From 31117fa7c70de9beb8aa39615e780214b251f519 Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Tue, 20 Sep 2022 12:46:22 +0200 Subject: 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 --- lib/private/Http/Client/LocalAddressChecker.php | 20 ++++++-------------- tests/lib/Http/Client/LocalAddressCheckerTest.php | 4 ++-- 2 files changed, 8 insertions(+), 16 deletions(-) (limited to 'tests/lib') diff --git a/lib/private/Http/Client/LocalAddressChecker.php b/lib/private/Http/Client/LocalAddressChecker.php index 274186d0a5a..13a7d062de3 100644 --- a/lib/private/Http/Client/LocalAddressChecker.php +++ b/lib/private/Http/Client/LocalAddressChecker.php @@ -25,6 +25,7 @@ declare(strict_types=1); */ namespace OC\Http\Client; +use IPLib\Address\IPv6; use IPLib\Factory; use IPLib\ParseStringFlag; use OCP\Http\Client\LocalServerException; @@ -48,7 +49,11 @@ class LocalAddressChecker { 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 @@ class LocalAddressChecker { $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 991801f043d..8c8e64eddf9 100644 --- a/tests/lib/Http/Client/LocalAddressCheckerTest.php +++ b/tests/lib/Http/Client/LocalAddressCheckerTest.php @@ -91,7 +91,7 @@ class LocalAddressCheckerTest extends \Test\TestCase { 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 @@ class LocalAddressCheckerTest extends \Test\TestCase { ['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'], -- cgit v1.2.3