Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Detect weird local ips #34160

Merged
merged 7 commits into from
Sep 22, 2022
Merged

Detect weird local ips #34160

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c083f77
Harden tests for local IP detection in URLs
come-nc Sep 20, 2022
a907b74
Add missing urldecode and idn_to_utf8 calls to local address checker
come-nc Sep 20, 2022
6b876bb
Add mlocati/ip-lib dependency
come-nc Sep 20, 2022
7ac688a
Use new dependency to normalize IPs
come-nc Sep 20, 2022
31117fa
Fix tests for nested v4 in v6
come-nc Sep 20, 2022
b3b98a3
Update 3rdparty to master
come-nc Sep 20, 2022
7532859
Fix idn_to_utf8 stub signature
come-nc Sep 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion 3rdparty
Open in desktop
Submodule 3rdparty updated 27 files
+1 −0 composer.json
+72 −1 composer.lock
+16 −0 composer/autoload_classmap.php
+1 −0 composer/autoload_psr4.php
+21 −0 composer/autoload_static.php
+74 −0 composer/installed.json
+11 −2 composer/installed.php
+20 −0 mlocati/ip-lib/LICENSE.txt
+648 −0 mlocati/ip-lib/README.md
+60 −0 mlocati/ip-lib/composer.json
+13 −0 mlocati/ip-lib/ip-lib.php
+156 −0 mlocati/ip-lib/src/Address/AddressInterface.php
+140 −0 mlocati/ip-lib/src/Address/AssignedRange.php
+515 −0 mlocati/ip-lib/src/Address/IPv4.php
+608 −0 mlocati/ip-lib/src/Address/IPv6.php
+44 −0 mlocati/ip-lib/src/Address/Type.php
+298 −0 mlocati/ip-lib/src/Factory.php
+79 −0 mlocati/ip-lib/src/ParseStringFlag.php
+125 −0 mlocati/ip-lib/src/Range/AbstractRange.php
+322 −0 mlocati/ip-lib/src/Range/Pattern.php
+160 −0 mlocati/ip-lib/src/Range/RangeInterface.php
+244 −0 mlocati/ip-lib/src/Range/Single.php
+354 −0 mlocati/ip-lib/src/Range/Subnet.php
+152 −0 mlocati/ip-lib/src/Range/Type.php
+120 −0 mlocati/ip-lib/src/Service/BinaryMath.php
+168 −0 mlocati/ip-lib/src/Service/RangesFromBoundaryCalculator.php
+171 −0 mlocati/ip-lib/src/Service/UnsignedIntegerMath.php
2 changes: 1 addition & 1 deletion lib/private/Http/Client/DnsPinMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public function addDnsPinning() {
$ports[] = (string)$port;
}

$targetIps = $this->dnsResolve($hostName, 0);
$targetIps = $this->dnsResolve(idn_to_utf8($hostName), 0);
Fixed Show fixed Hide fixed

$curlResolves = [];

Expand Down
33 changes: 19 additions & 14 deletions lib/private/Http/Client/LocalAddressChecker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
*/
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;
Expand All @@ -37,6 +40,21 @@ 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 */
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
Expand All @@ -50,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 {
Expand All @@ -72,7 +77,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)));
Fixed Show fixed Hide fixed
// Remove brackets from IPv6 addresses
if (strpos($host, '[') === 0 && substr($host, -1) === ']') {
$host = substr($host, 1, -1);
Expand Down
1 change: 1 addition & 0 deletions tests/lib/Http/Client/ClientTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ public function dataPreventLocalAddress():array {
['another-host.local'],
['service.localhost'],
['!@#$'], // test invalid url
['normal.host.com'],
];
}

Expand Down
23 changes: 21 additions & 2 deletions tests/lib/Http/Client/LocalAddressCheckerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
['::'],
Expand All @@ -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'],
Expand All @@ -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]'],
];
}

Expand Down