Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
102 changes: 97 additions & 5 deletions app/Rules/PhotoUrlRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,17 @@

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');
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/**
Expand Down Expand Up @@ -83,23 +91,107 @@ 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;
Comment thread
ildyria marked this conversation as resolved.
}

/**
* 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;
}

foreach ($ips as $ip) {
if (str_starts_with($ip, '127.') || $ip === '::1') {
return true;
}
}

return false;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
52 changes: 52 additions & 0 deletions database/migrations/2026_03_22_094636_bump_version070502.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

/**
* SPDX-License-Identifier: MIT
* Copyright (c) 2017-2018 Tobias Reich
* Copyright (c) 2018-2026 LycheeOrg.
*/

use Illuminate\Database\Migrations\Migration;
use Illuminate\Support\Facades\Artisan;
use Illuminate\Support\Facades\DB;
use Symfony\Component\Console\Output\ConsoleOutput;
use Symfony\Component\Console\Output\ConsoleSectionOutput;

return new class() extends Migration {
private ConsoleOutput $output;
private ConsoleSectionOutput $msg_section;

public function __construct()
{
$this->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('<error>Warning:</error> Failed to clear cache for version 7.5.2');

return;
}
$this->msg_section->writeln('<info>Info:</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']);
}
};
64 changes: 59 additions & 5 deletions tests/Unit/Rules/PhotoUrlRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
namespace Tests\Unit\Rules;

use App\Models\Configs;
use App\Repositories\ConfigManager;
use App\Rules\PhotoUrlRule;
use Tests\AbstractTestCase;

Expand All @@ -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 = '';

Expand All @@ -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');
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -149,17 +169,51 @@ 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);
}

/**
* Test validation when localhost is forbidden and localhost is provided.
*/
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' => '127.0.0.1']],
Comment thread
ildyria marked this conversation as resolved.
Outdated
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);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion version.md
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7.5.1
7.5.2
Loading