diff --git a/app/Rules/PhotoUrlRule.php b/app/Rules/PhotoUrlRule.php index e159105308d..d979d8b86d1 100644 --- a/app/Rules/PhotoUrlRule.php +++ b/app/Rules/PhotoUrlRule.php @@ -10,14 +10,24 @@ use App\Repositories\ConfigManager; use Illuminate\Contracts\Validation\ValidationRule; +use Safe\Exceptions\NetworkException; use Safe\Exceptions\UrlException; +use function Safe\inet_pton; use function Safe\parse_url; final class PhotoUrlRule implements ValidationRule { + /** + * @param ConfigManager $config_manager + * @param \Closure $dns_get_record defaulted to dns_get_record(string $hostname, int $type = ?, array &$authoritative_name_servers = ?, array &$additional_records = ?, bool $raw = ?): array|false + * + * @return void + */ public function __construct( private ConfigManager $config_manager, + private \Closure|null $dns_get_record = null, ) { + $this->dns_get_record = $dns_get_record ?? \Closure::fromCallable('dns_get_record'); } /** @@ -83,23 +93,117 @@ public function validate(string $attribute, mixed $value, \Closure $fail): void return; } + $resolved_ips = $this->resolveHostToIPs($host); + if ( $this->config_manager->getValueAsBool('import_via_url_forbidden_local_ip') && - filter_var($host, FILTER_VALIDATE_IP) !== false && - filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) === false + $this->hasPrivateOrReservedIP($resolved_ips) ) { - $fail($attribute . ' must not be a private IP address.'); + $fail($attribute . ' must not resolve to a private or reserved IP address.'); return; } if ( $this->config_manager->getValueAsBool('import_via_url_forbidden_localhost') && - in_array(strtolower($host), ['localhost', '127.0.0.1', '::1'], true) + $this->hasLocalhostIP($host, $resolved_ips) ) { - $fail($attribute . ' must not be localhost.'); + $fail($attribute . ' must not resolve to localhost.'); return; } } + + /** + * Resolve a hostname to its IP addresses. + * + * If the host is already an IP address, return it directly. + * + * @param string $host + * + * @return string[] + */ + private function resolveHostToIPs(string $host): array + { + // If the host is already a valid IP, no resolution needed. + if (filter_var($host, FILTER_VALIDATE_IP) !== false) { + return [$host]; + } + + $ips = []; + + try { + // Resolve A records (IPv4). + $a_records = call_user_func($this->dns_get_record, $host, DNS_A); + if ($a_records !== false) { + foreach ($a_records as $record) { + $ips[] = $record['ip']; + } + } + + // Resolve AAAA records (IPv6). + $aaaa_records = call_user_func($this->dns_get_record, $host, DNS_AAAA); + if ($aaaa_records !== false) { + foreach ($aaaa_records as $record) { + $ips[] = $record['ipv6']; + } + } + } catch (\ErrorException) { + // DNS resolution failed — return empty array. + // The hostname checks (e.g. literal "localhost") still apply. + } + + return $ips; + } + + /** + * Check if any of the resolved IPs are private or reserved. + * + * @param string[] $ips + * + * @return bool + */ + private function hasPrivateOrReservedIP(array $ips): bool + { + foreach ($ips as $ip) { + if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) === false) { + return true; + } + } + + return false; + } + + /** + * Check if the host or any resolved IP is localhost. + * + * @param string $host + * @param string[] $ips + * + * @return bool + */ + private function hasLocalhostIP(string $host, array $ips): bool + { + if (strtolower($host) === 'localhost') { + return true; + } + + $loopback_v6 = inet_pton('::1'); + foreach ($ips as $ip) { + if (str_starts_with($ip, '127.')) { + return true; + } + + try { + $ip = inet_pton($ip); + if ($ip === $loopback_v6) { + return true; + } + } catch (NetworkException) { + return true; // If we can't parse the IP, assume it's invalid and potentially localhost. + } + } + + return false; + } } diff --git a/database/migrations/2026_03_22_094636_bump_version070502.php b/database/migrations/2026_03_22_094636_bump_version070502.php new file mode 100644 index 00000000000..56eb5d8f9b5 --- /dev/null +++ b/database/migrations/2026_03_22_094636_bump_version070502.php @@ -0,0 +1,52 @@ +output = new ConsoleOutput(); + $this->msg_section = $this->output->section(); + } + + /** + * Run the migrations. + * + * @return void + */ + public function up(): void + { + DB::table('configs')->where('key', 'version')->update(['value' => '070502']); + try { + Artisan::call('cache:clear'); + } catch (\Throwable $e) { + $this->msg_section->writeln('Warning: Failed to clear cache for version 7.5.2'); + + return; + } + $this->msg_section->writeln('Info: Cleared cache for version 7.5.2'); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down(): void + { + DB::table('configs')->where('key', 'version')->update(['value' => '070501']); + } +}; diff --git a/tests/Unit/Rules/PhotoUrlRuleTest.php b/tests/Unit/Rules/PhotoUrlRuleTest.php index 6fd47f8db7f..edc7d15f35b 100644 --- a/tests/Unit/Rules/PhotoUrlRuleTest.php +++ b/tests/Unit/Rules/PhotoUrlRuleTest.php @@ -19,6 +19,7 @@ namespace Tests\Unit\Rules; use App\Models\Configs; +use App\Repositories\ConfigManager; use App\Rules\PhotoUrlRule; use Tests\AbstractTestCase; @@ -31,7 +32,7 @@ class PhotoUrlRuleTest extends AbstractTestCase public function setUp(): void { parent::setUp(); - $this->rule = resolve(PhotoUrlRule::class); + $this->rule = $this->makeRule(); $this->failCalled = false; $this->failMessage = ''; @@ -41,6 +42,19 @@ public function setUp(): void Configs::set('import_via_url_forbidden_localhost', '1'); } + /** + * Create a PhotoUrlRule with an optional mock DNS resolver. + * + * @param \Closure|null $dns_get_record + */ + private function makeRule(?\Closure $dns_get_record = null): PhotoUrlRule + { + return new PhotoUrlRule( + resolve(ConfigManager::class), + $dns_get_record ?? fn (string $hostname, int $type = DNS_A) => [], + ); + } + public function tearDown(): void { Configs::set('import_via_url_require_https', '1'); @@ -102,6 +116,10 @@ public function testHttpsRequiredButNotProvided(): void */ public function testHttpsRequiredAndProvided(): void { + $this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) { + DNS_A => [['ip' => '93.184.216.34']], + default => [], + }); $this->rule->validate('photo_url', 'https://example.com', fn ($m) => $this->m($m)); self::assertFalse($this->failCalled); self::assertEquals('', $this->failMessage); @@ -134,9 +152,11 @@ public function testForbiddenPort(): void */ public function testAllowedPort(): void { - $this->rule->validate('photo_url', 'https://example.com:80', function ($message) { - $this->failCalled = true; + $this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) { + DNS_A => [['ip' => '93.184.216.34']], + default => [], }); + $this->rule->validate('photo_url', 'https://example.com:80', fn ($m) => $this->m($m)); self::assertFalse($this->failCalled); self::assertEquals('', $this->failMessage); @@ -149,7 +169,7 @@ public function testForbiddenPrivateIp(): void { $this->rule->validate('photo_url', 'https://192.168.1.1', fn ($m) => $this->m($m)); self::assertTrue($this->failCalled); - self::assertEquals('photo_url must not be a private IP address.', $this->failMessage); + self::assertEquals('photo_url must not resolve to a private or reserved IP address.', $this->failMessage); } /** @@ -157,9 +177,43 @@ public function testForbiddenPrivateIp(): void */ public function testForbiddenLocalhost(): void { + // Disable private IP check so we specifically test the localhost check. + Configs::set('import_via_url_forbidden_local_ip', '0'); + $this->rule->validate('photo_url', 'https://localhost', fn ($m) => $this->m($m)); self::assertTrue($this->failCalled); - self::assertEquals('photo_url must not be localhost.', $this->failMessage); + self::assertEquals('photo_url must not resolve to localhost.', $this->failMessage); + } + + /** + * Test that hostnames resolving to private IPs are blocked (DNS rebinding protection). + */ + public function testForbiddenPrivateIpViaHostname(): void + { + $this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) { + DNS_A => [['ip' => '192.168.0.1']], + default => [], + }); + $this->rule->validate('photo_url', 'https://evil.example.com/test.jpg', fn ($m) => $this->m($m)); + self::assertTrue($this->failCalled); + self::assertEquals('photo_url must not resolve to a private or reserved IP address.', $this->failMessage); + } + + /** + * Test that hostnames resolving to localhost are blocked. + */ + public function testForbiddenLocalhostViaHostname(): void + { + $this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) { + DNS_A => [['ip' => '127.0.0.1']], + default => [], + }); + + Configs::set('import_via_url_forbidden_local_ip', '0'); + + $this->rule->validate('photo_url', 'https://evil.example.com/test.jpg', fn ($m) => $this->m($m)); + self::assertTrue($this->failCalled); + self::assertEquals('photo_url must not resolve to localhost.', $this->failMessage); } /** diff --git a/version.md b/version.md index 7501d508f74..26fb97caff7 100644 --- a/version.md +++ b/version.md @@ -1 +1 @@ -7.5.1 \ No newline at end of file +7.5.2 \ No newline at end of file