Merge pull request #34160 from nextcloud/fix/detect-weird-local-ips

Detect weird local ips

3rdparty

@@ -1 +1 @@
-Subproject commit f143482ffb0b8dfdbc08cd848ce2e66f02a5d9b6
+Subproject commit 3095d4062823f3f913d594f9ff313010ed55cd74

build/stubs/intl.php

@@ -4622,7 +4622,7 @@ function idn_to_ascii($domain, $options = 0, $variant = INTL_IDNA_VARIANT_2003,
  * @param int $variant [optional] <p>
  * Either INTL_IDNA_VARIANT_2003 for IDNA 2003 or INTL_IDNA_VARIANT_UTS46 for UTS #46.
  * </p>
- * @param int &$idna_info [optional] <p>
+ * @param array &$idna_info [optional] <p>
  * 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 &gt;=5.5.0 PECL intl &gt;= 3.0.0a1)<br/>

lib/private/Http/Client/DnsPinMiddleware.php

@@ -125,7 +125,7 @@ class DnsPinMiddleware {
 					$ports[] = (string)$port;
 				}
 
-				$targetIps = $this->dnsResolve($hostName, 0);
+				$targetIps = $this->dnsResolve(idn_to_utf8($hostName), 0);
 
 				$curlResolves = [];
 

lib/private/Http/Client/LocalAddressChecker.php

@@ -25,6 +25,9 @@ declare(strict_types=1);
  */
 namespace OC\Http\Client;
 
+use IPLib\Address\IPv6;
+use IPLib\Factory;
+use IPLib\ParseStringFlag;
 use OCP\Http\Client\LocalServerException;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\IpUtils;
@@ -37,6 +40,21 @@ class LocalAddressChecker {
 	}
 
 	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 */
+		if ($parsedIp instanceof IPv6) {
+			$ip = (string)($parsedIp->toIPv4() ?? $parsedIp);
+		} else {
+			$ip = (string)$parsedIp;
+		}
+
 		$localRanges = [
 			'100.64.0.0/10', // See RFC 6598
 			'192.0.0.0/24', // See RFC 6890
@@ -50,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 {
@@ -72,7 +77,7 @@ class LocalAddressChecker {
 			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);

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'],
 		];
 	}
 

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'],
@@ -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]'],
 		];
 	}