Skip to content

Commit 28c5261

Browse files
ildyriaildyria
authored
Fix dns-resolving to localip (#4214)
1 parent 9ad0993 commit 28c5261

File tree

4 files changed

+221
-11
lines changed

4 files changed

+221
-11
lines changed

‎app/Rules/PhotoUrlRule.php‎

Lines changed: 109 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,24 @@
1010

1111
use App\Repositories\ConfigManager;
1212
use Illuminate\Contracts\Validation\ValidationRule;
13+
use Safe\Exceptions\NetworkException;
1314
use Safe\Exceptions\UrlException;
15+
use function Safe\inet_pton;
1416
use function Safe\parse_url;
1517

1618
final class PhotoUrlRule implements ValidationRule
1719
{
20+
/**
21+
* @param ConfigManager $config_manager
22+
* @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
23+
*
24+
* @return void
25+
*/
1826
public function __construct(
1927
private ConfigManager $config_manager,
28+
private \Closure|null $dns_get_record = null,
2029
) {
30+
$this->dns_get_record = $dns_get_record ?? \Closure::fromCallable('dns_get_record');
2131
}
2232

2333
/**
@@ -83,23 +93,117 @@ public function validate(string $attribute, mixed $value, \Closure $fail): void
8393
return;
8494
}
8595

96+
$resolved_ips = $this->resolveHostToIPs($host);
97+
8698
if (
8799
$this->config_manager->getValueAsBool('import_via_url_forbidden_local_ip') &&
88-
filter_var($host, FILTER_VALIDATE_IP) !== false &&
89-
filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) === false
100+
$this->hasPrivateOrReservedIP($resolved_ips)
90101
) {
91-
$fail($attribute . ' must not be a private IP address.');
102+
$fail($attribute . ' must not resolve to a private or reserved IP address.');
92103

93104
return;
94105
}
95106

96107
if (
97108
$this->config_manager->getValueAsBool('import_via_url_forbidden_localhost') &&
98-
in_array(strtolower($host), ['localhost', '127.0.0.1', '::1'], true)
109+
$this->hasLocalhostIP($host, $resolved_ips)
99110
) {
100-
$fail($attribute . ' must not be localhost.');
111+
$fail($attribute . ' must not resolve to localhost.');
101112

102113
return;
103114
}
104115
}
116+
117+
/**
118+
* Resolve a hostname to its IP addresses.
119+
*
120+
* If the host is already an IP address, return it directly.
121+
*
122+
* @param string $host
123+
*
124+
* @return string[]
125+
*/
126+
private function resolveHostToIPs(string $host): array
127+
{
128+
// If the host is already a valid IP, no resolution needed.
129+
if (filter_var($host, FILTER_VALIDATE_IP) !== false) {
130+
return [$host];
131+
}
132+
133+
$ips = [];
134+
135+
try {
136+
// Resolve A records (IPv4).
137+
$a_records = call_user_func($this->dns_get_record, $host, DNS_A);
138+
if ($a_records !== false) {
139+
foreach ($a_records as $record) {
140+
$ips[] = $record['ip'];
141+
}
142+
}
143+
144+
// Resolve AAAA records (IPv6).
145+
$aaaa_records = call_user_func($this->dns_get_record, $host, DNS_AAAA);
146+
if ($aaaa_records !== false) {
147+
foreach ($aaaa_records as $record) {
148+
$ips[] = $record['ipv6'];
149+
}
150+
}
151+
} catch (\ErrorException) {
152+
// DNS resolution failed — return empty array.
153+
// The hostname checks (e.g. literal "localhost") still apply.
154+
}
155+
156+
return $ips;
157+
}
158+
159+
/**
160+
* Check if any of the resolved IPs are private or reserved.
161+
*
162+
* @param string[] $ips
163+
*
164+
* @return bool
165+
*/
166+
private function hasPrivateOrReservedIP(array $ips): bool
167+
{
168+
foreach ($ips as $ip) {
169+
if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) === false) {
170+
return true;
171+
}
172+
}
173+
174+
return false;
175+
}
176+
177+
/**
178+
* Check if the host or any resolved IP is localhost.
179+
*
180+
* @param string $host
181+
* @param string[] $ips
182+
*
183+
* @return bool
184+
*/
185+
private function hasLocalhostIP(string $host, array $ips): bool
186+
{
187+
if (strtolower($host) === 'localhost') {
188+
return true;
189+
}
190+
191+
$loopback_v6 = inet_pton('::1');
192+
foreach ($ips as $ip) {
193+
if (str_starts_with($ip, '127.')) {
194+
return true;
195+
}
196+
197+
try {
198+
$ip = inet_pton($ip);
199+
if ($ip === $loopback_v6) {
200+
return true;
201+
}
202+
} catch (NetworkException) {
203+
return true; // If we can't parse the IP, assume it's invalid and potentially localhost.
204+
}
205+
}
206+
207+
return false;
208+
}
105209
}

‎database/migrations/2026_03_22_094636_bump_version070502.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
/**
4+
* SPDX-License-Identifier: MIT
5+
* Copyright (c) 2017-2018 Tobias Reich
6+
* Copyright (c) 2018-2026 LycheeOrg.
7+
*/
8+
9+
use Illuminate\Database\Migrations\Migration;
10+
use Illuminate\Support\Facades\Artisan;
11+
use Illuminate\Support\Facades\DB;
12+
use Symfony\Component\Console\Output\ConsoleOutput;
13+
use Symfony\Component\Console\Output\ConsoleSectionOutput;
14+
15+
return new class() extends Migration {
16+
private ConsoleOutput $output;
17+
private ConsoleSectionOutput $msg_section;
18+
19+
public function __construct()
20+
{
21+
$this->output = new ConsoleOutput();
22+
$this->msg_section = $this->output->section();
23+
}
24+
25+
/**
26+
* Run the migrations.
27+
*
28+
* @return void
29+
*/
30+
public function up(): void
31+
{
32+
DB::table('configs')->where('key', 'version')->update(['value' => '070502']);
33+
try {
34+
Artisan::call('cache:clear');
35+
} catch (\Throwable $e) {
36+
$this->msg_section->writeln('<error>Warning:</error> Failed to clear cache for version 7.5.2');
37+
38+
return;
39+
}
40+
$this->msg_section->writeln('<info>Info:</info> Cleared cache for version 7.5.2');
41+
}
42+
43+
/**
44+
* Reverse the migrations.
45+
*
46+
* @return void
47+
*/
48+
public function down(): void
49+
{
50+
DB::table('configs')->where('key', 'version')->update(['value' => '070501']);
51+
}
52+
};

‎tests/Unit/Rules/PhotoUrlRuleTest.php‎

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
namespace Tests\Unit\Rules;
2020

2121
use App\Models\Configs;
22+
use App\Repositories\ConfigManager;
2223
use App\Rules\PhotoUrlRule;
2324
use Tests\AbstractTestCase;
2425

@@ -31,7 +32,7 @@ class PhotoUrlRuleTest extends AbstractTestCase
3132
public function setUp(): void
3233
{
3334
parent::setUp();
34-
$this->rule = resolve(PhotoUrlRule::class);
35+
$this->rule = $this->makeRule();
3536
$this->failCalled = false;
3637
$this->failMessage = '';
3738

@@ -41,6 +42,19 @@ public function setUp(): void
4142
Configs::set('import_via_url_forbidden_localhost', '1');
4243
}
4344

45+
/**
46+
* Create a PhotoUrlRule with an optional mock DNS resolver.
47+
*
48+
* @param \Closure|null $dns_get_record
49+
*/
50+
private function makeRule(?\Closure $dns_get_record = null): PhotoUrlRule
51+
{
52+
return new PhotoUrlRule(
53+
resolve(ConfigManager::class),
54+
$dns_get_record ?? fn (string $hostname, int $type = DNS_A) => [],
55+
);
56+
}
57+
4458
public function tearDown(): void
4559
{
4660
Configs::set('import_via_url_require_https', '1');
@@ -102,6 +116,10 @@ public function testHttpsRequiredButNotProvided(): void
102116
*/
103117
public function testHttpsRequiredAndProvided(): void
104118
{
119+
$this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) {
120+
DNS_A => [['ip' => '93.184.216.34']],
121+
default => [],
122+
});
105123
$this->rule->validate('photo_url', 'https://example.com', fn ($m) => $this->m($m));
106124
self::assertFalse($this->failCalled);
107125
self::assertEquals('', $this->failMessage);
@@ -134,9 +152,11 @@ public function testForbiddenPort(): void
134152
*/
135153
public function testAllowedPort(): void
136154
{
137-
$this->rule->validate('photo_url', 'https://example.com:80', function ($message) {
138-
$this->failCalled = true;
155+
$this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) {
156+
DNS_A => [['ip' => '93.184.216.34']],
157+
default => [],
139158
});
159+
$this->rule->validate('photo_url', 'https://example.com:80', fn ($m) => $this->m($m));
140160

141161
self::assertFalse($this->failCalled);
142162
self::assertEquals('', $this->failMessage);
@@ -149,17 +169,51 @@ public function testForbiddenPrivateIp(): void
149169
{
150170
$this->rule->validate('photo_url', 'https://192.168.1.1', fn ($m) => $this->m($m));
151171
self::assertTrue($this->failCalled);
152-
self::assertEquals('photo_url must not be a private IP address.', $this->failMessage);
172+
self::assertEquals('photo_url must not resolve to a private or reserved IP address.', $this->failMessage);
153173
}
154174

155175
/**
156176
* Test validation when localhost is forbidden and localhost is provided.
157177
*/
158178
public function testForbiddenLocalhost(): void
159179
{
180+
// Disable private IP check so we specifically test the localhost check.
181+
Configs::set('import_via_url_forbidden_local_ip', '0');
182+
160183
$this->rule->validate('photo_url', 'https://localhost', fn ($m) => $this->m($m));
161184
self::assertTrue($this->failCalled);
162-
self::assertEquals('photo_url must not be localhost.', $this->failMessage);
185+
self::assertEquals('photo_url must not resolve to localhost.', $this->failMessage);
186+
}
187+
188+
/**
189+
* Test that hostnames resolving to private IPs are blocked (DNS rebinding protection).
190+
*/
191+
public function testForbiddenPrivateIpViaHostname(): void
192+
{
193+
$this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) {
194+
DNS_A => [['ip' => '192.168.0.1']],
195+
default => [],
196+
});
197+
$this->rule->validate('photo_url', 'https://evil.example.com/test.jpg', fn ($m) => $this->m($m));
198+
self::assertTrue($this->failCalled);
199+
self::assertEquals('photo_url must not resolve to a private or reserved IP address.', $this->failMessage);
200+
}
201+
202+
/**
203+
* Test that hostnames resolving to localhost are blocked.
204+
*/
205+
public function testForbiddenLocalhostViaHostname(): void
206+
{
207+
$this->rule = $this->makeRule(fn (string $hostname, int $type = DNS_A) => match ($type) {
208+
DNS_A => [['ip' => '127.0.0.1']],
209+
default => [],
210+
});
211+
212+
Configs::set('import_via_url_forbidden_local_ip', '0');
213+
214+
$this->rule->validate('photo_url', 'https://evil.example.com/test.jpg', fn ($m) => $this->m($m));
215+
self::assertTrue($this->failCalled);
216+
self::assertEquals('photo_url must not resolve to localhost.', $this->failMessage);
163217
}
164218

165219
/**

‎version.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
7.5.1
1+
7.5.2

0 commit comments

Comments
 (0)