Skip to content

Fix IP range null safety and add security IP tests #58369

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

Open
amussara wants to merge 1 commit into
base: master
Choose a base branch
from
+355 −1
Open

Fix IP range null safety and add security IP tests #58369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion lib/private/Security/Ip/Range.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ public static function isValid(string $range): bool {
}

public function contains(IAddress $address): bool {
return $this->range->contains(Factory::parseAddressString((string)$address, ParseStringFlag::MAY_INCLUDE_ZONEID));
$parsedAddress = Factory::parseAddressString((string)$address, ParseStringFlag::MAY_INCLUDE_ZONEID);
if ($parsedAddress === null) {
return false;
}
return $this->range->contains($parsedAddress);
}

public function __toString(): string {
Expand Down
133 changes: 133 additions & 0 deletions tests/lib/Security/Ip/AddressTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace Test\Security\Ip;

use InvalidArgumentException;
use OC\Security\Ip\Address;
use OC\Security\Ip\Range;
use OCP\Security\Ip\IAddress;

class AddressTest extends \Test\TestCase {
public function testImplementsInterface(): void {
$address = new Address('127.0.0.1');
$this->assertInstanceOf(IAddress::class, $address);
}

#[\PHPUnit\Framework\Attributes\DataProvider('validAddressProvider')]
public function testConstructorWithValidAddress(string $ip): void {
$address = new Address($ip);
$this->assertNotEmpty((string)$address);
}

public static function validAddressProvider(): array {
return [
'IPv4 loopback' => ['127.0.0.1'],
'IPv4 private' => ['192.168.1.1'],
'IPv4 public' => ['8.8.8.8'],
'IPv4 zero' => ['0.0.0.0'],
'IPv4 broadcast' => ['255.255.255.255'],
'IPv6 loopback' => ['::1'],
'IPv6 full' => ['2001:0db8:85a3:0000:0000:8a2e:0370:7334'],
'IPv6 compressed' => ['2001:db8::1'],
'IPv6 link-local' => ['fe80::1'],
'IPv6 with zone ID' => ['fe80::1%eth0'],
'IPv6 mapped IPv4' => ['::ffff:192.168.1.1'],
];
}

#[\PHPUnit\Framework\Attributes\DataProvider('invalidAddressProvider')]
public function testConstructorWithInvalidAddress(string $ip): void {
$this->expectException(InvalidArgumentException::class);
new Address($ip);
}

public static function invalidAddressProvider(): array {
return [
'empty string' => [''],
'random text' => ['not-an-ip'],
'incomplete IPv4' => ['192.168.1'],
'IPv4 out of range' => ['256.256.256.256'],
'CIDR notation' => ['192.168.1.0/24'],
'IPv4 with port' => ['192.168.1.1:8080'],
];
}

#[\PHPUnit\Framework\Attributes\DataProvider('isValidProvider')]
public function testIsValid(string $ip, bool $expected): void {
$this->assertSame($expected, Address::isValid($ip));
}

public static function isValidProvider(): array {
return [
['127.0.0.1', true],
['::1', true],
['192.168.1.1', true],
['2001:db8::1', true],
['fe80::1%eth0', true],
['', false],
['not-an-ip', false],
['256.1.2.3', false],
['192.168.1.0/24', false],
];
}

public function testToString(): void {
$address = new Address('127.0.0.1');
$this->assertSame('127.0.0.1', (string)$address);
}

public function testToStringIPv6Normalized(): void {
$address = new Address('2001:0db8:0000:0000:0000:0000:0000:0001');
$this->assertSame('2001:db8::1', (string)$address);
}

public function testMatchesReturnsTrueWhenInRange(): void {
$address = new Address('192.168.1.100');
$range = new Range('192.168.1.0/24');
$this->assertTrue($address->matches($range));
}

public function testMatchesReturnsFalseWhenNotInRange(): void {
$address = new Address('10.0.0.1');
$range = new Range('192.168.1.0/24');
$this->assertFalse($address->matches($range));
}

public function testMatchesWithMultipleRanges(): void {
$address = new Address('10.0.0.5');
$range1 = new Range('192.168.1.0/24');
$range2 = new Range('10.0.0.0/8');
$this->assertTrue($address->matches($range1, $range2));
}

public function testMatchesWithNoRanges(): void {
$address = new Address('192.168.1.1');
$this->assertFalse($address->matches());
}

public function testMatchesWithMultipleRangesNoneMatching(): void {
$address = new Address('172.16.0.1');
$range1 = new Range('192.168.1.0/24');
$range2 = new Range('10.0.0.0/8');
$this->assertFalse($address->matches($range1, $range2));
}

public function testMatchesIPv6InRange(): void {
$address = new Address('2001:db8::1');
$range = new Range('2001:db8::/32');
$this->assertTrue($address->matches($range));
}

public function testMatchesIPv6NotInRange(): void {
$address = new Address('2001:db9::1');
$range = new Range('2001:db8::/32');
$this->assertFalse($address->matches($range));
}
}
93 changes: 93 additions & 0 deletions tests/lib/Security/Ip/FactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace Test\Security\Ip;

use InvalidArgumentException;
use OC\Security\Ip\Factory;
use OCP\Security\Ip\IAddress;
use OCP\Security\Ip\IFactory;
use OCP\Security\Ip\IRange;

class FactoryTest extends \Test\TestCase {
private Factory $factory;

protected function setUp(): void {
parent::setUp();
$this->factory = new Factory();
}

public function testImplementsInterface(): void {
$this->assertInstanceOf(IFactory::class, $this->factory);
}

public function testRangeFromStringReturnsIRange(): void {
$range = $this->factory->rangeFromString('192.168.1.0/24');
$this->assertInstanceOf(IRange::class, $range);
}

public function testAddressFromStringReturnsIAddress(): void {
$address = $this->factory->addressFromString('192.168.1.1');
$this->assertInstanceOf(IAddress::class, $address);
}

public function testRangeFromStringWithIPv4(): void {
$range = $this->factory->rangeFromString('10.0.0.0/8');
$this->assertSame('10.0.0.0/8', (string)$range);
}

public function testRangeFromStringWithIPv6(): void {
$range = $this->factory->rangeFromString('2001:db8::/32');
$this->assertSame('2001:db8::/32', (string)$range);
}

public function testAddressFromStringWithIPv4(): void {
$address = $this->factory->addressFromString('127.0.0.1');
$this->assertSame('127.0.0.1', (string)$address);
}

public function testAddressFromStringWithIPv6(): void {
$address = $this->factory->addressFromString('::1');
$this->assertSame('::1', (string)$address);
}

public function testRangeFromStringWithInvalidRange(): void {
$this->expectException(InvalidArgumentException::class);
$this->factory->rangeFromString('not-a-range');
}

public function testAddressFromStringWithInvalidAddress(): void {
$this->expectException(InvalidArgumentException::class);
$this->factory->addressFromString('not-an-ip');
}

public function testCreatedRangeContainsCreatedAddress(): void {
$range = $this->factory->rangeFromString('192.168.1.0/24');
$address = $this->factory->addressFromString('192.168.1.50');
$this->assertTrue($range->contains($address));
}

public function testCreatedRangeDoesNotContainOutsideAddress(): void {
$range = $this->factory->rangeFromString('192.168.1.0/24');
$address = $this->factory->addressFromString('10.0.0.1');
$this->assertFalse($range->contains($address));
}

public function testCreatedAddressMatchesCreatedRange(): void {
$range = $this->factory->rangeFromString('10.0.0.0/8');
$address = $this->factory->addressFromString('10.5.3.2');
$this->assertTrue($address->matches($range));
}

public function testRangeFromStringWithWildcard(): void {
$range = $this->factory->rangeFromString('192.168.1.*');
$address = $this->factory->addressFromString('192.168.1.123');
$this->assertTrue($range->contains($address));
}
}
124 changes: 124 additions & 0 deletions tests/lib/Security/Ip/RangeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace Test\Security\Ip;

use InvalidArgumentException;
use OC\Security\Ip\Address;
use OC\Security\Ip\Range;
use OCP\Security\Ip\IRange;

class RangeTest extends \Test\TestCase {
public function testImplementsInterface(): void {
$range = new Range('192.168.1.0/24');
$this->assertInstanceOf(IRange::class, $range);
}

#[\PHPUnit\Framework\Attributes\DataProvider('validRangeProvider')]
public function testConstructorWithValidRange(string $range): void {
$rangeObj = new Range($range);
$this->assertNotEmpty((string)$rangeObj);
}

public static function validRangeProvider(): array {
return [
'IPv4 CIDR /24' => ['192.168.1.0/24'],
'IPv4 CIDR /32' => ['10.0.0.1/32'],
'IPv4 CIDR /0' => ['0.0.0.0/0'],
'IPv4 CIDR /16' => ['172.16.0.0/16'],
'IPv4 CIDR /8' => ['10.0.0.0/8'],
'IPv4 single address' => ['192.168.1.1'],
'IPv4 wildcard' => ['192.168.1.*'],
'IPv6 CIDR /64' => ['2001:db8::/64'],
'IPv6 CIDR /128' => ['::1/128'],
'IPv6 CIDR /32' => ['2001:db8::/32'],
'IPv6 single address' => ['::1'],
'IPv6 full notation' => ['2001:0db8:85a3:0000:0000:8a2e:0370:7334'],
];
}

#[\PHPUnit\Framework\Attributes\DataProvider('invalidRangeProvider')]
public function testConstructorWithInvalidRange(string $range): void {
$this->expectException(InvalidArgumentException::class);
new Range($range);
}

public static function invalidRangeProvider(): array {
return [
'empty string' => [''],
'random text' => ['not-a-range'],
'invalid CIDR' => ['192.168.1.0/33'],
'negative CIDR' => ['192.168.1.0/-1'],
'IPv4 out of range' => ['256.256.256.256/24'],
'malformed' => ['192.168/24'],
];
}

#[\PHPUnit\Framework\Attributes\DataProvider('isValidProvider')]
public function testIsValid(string $range, bool $expected): void {
$this->assertSame($expected, Range::isValid($range));
}

public static function isValidProvider(): array {
return [
['192.168.1.0/24', true],
['10.0.0.0/8', true],
['::1/128', true],
['2001:db8::/32', true],
['192.168.1.*', true],
['192.168.1.1', true],
['', false],
['not-a-range', false],
['192.168.1.0/33', false],
];
}

#[\PHPUnit\Framework\Attributes\DataProvider('containsProvider')]
public function testContains(string $range, string $address, bool $expected): void {
$rangeObj = new Range($range);
$addressObj = new Address($address);
$this->assertSame($expected, $rangeObj->contains($addressObj));
}

public static function containsProvider(): array {
return [
'IPv4 in /24' => ['192.168.1.0/24', '192.168.1.100', true],
'IPv4 first in /24' => ['192.168.1.0/24', '192.168.1.0', true],
'IPv4 last in /24' => ['192.168.1.0/24', '192.168.1.255', true],
'IPv4 outside /24' => ['192.168.1.0/24', '192.168.2.1', false],
'IPv4 in /32' => ['10.0.0.1/32', '10.0.0.1', true],
'IPv4 outside /32' => ['10.0.0.1/32', '10.0.0.2', false],
'IPv4 in /8' => ['10.0.0.0/8', '10.255.255.255', true],
'IPv4 outside /8' => ['10.0.0.0/8', '11.0.0.1', false],
'IPv4 wildcard match' => ['192.168.1.*', '192.168.1.50', true],
'IPv4 wildcard no match' => ['192.168.1.*', '192.168.2.50', false],
'IPv6 in /64' => ['2001:db8::/64', '2001:db8::ffff', true],
'IPv6 outside /64' => ['2001:db8::/64', '2001:db9::1', false],
'IPv6 in /128' => ['::1/128', '::1', true],
'IPv6 outside /128' => ['::1/128', '::2', false],
'IPv4 match all' => ['0.0.0.0/0', '192.168.1.1', true],
'IPv4 loopback in range' => ['127.0.0.0/8', '127.0.0.1', true],
];
}

public function testToString(): void {
$range = new Range('192.168.1.0/24');
$this->assertSame('192.168.1.0/24', (string)$range);
}

public function testToStringNormalizesIPv6(): void {
$range = new Range('2001:0db8:0000:0000:0000:0000:0000:0000/32');
$this->assertSame('2001:db8::/32', (string)$range);
}

public function testToStringSingleIPv4(): void {
$range = new Range('192.168.1.1');
$this->assertSame('192.168.1.1', (string)$range);
}
}