From c7f014017b753a53e33a4eb7d2950f7302d971b5 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 2 Jul 2026 14:46:46 +0200 Subject: [PATCH] Improve outbound URL validation --- app/Jobs/SendMessageToDiscordJob.php | 19 +- app/Jobs/SendMessageToSlackJob.php | 21 +- app/Jobs/SendWebhookJob.php | 2 +- app/Models/S3Storage.php | 1 + app/Rules/SafeExternalUrl.php | 146 ++++++++++--- app/Rules/SafeWebhookUrl.php | 191 ++++++++++++++---- tests/Unit/NotificationWebhookSafetyTest.php | 36 ++++ .../Unit/S3StorageEndpointValidationTest.php | 6 +- tests/Unit/S3StorageTest.php | 2 + tests/Unit/SafeExternalUrlTest.php | 41 +++- tests/Unit/SafeWebhookUrlTest.php | 29 +++ tests/Unit/SendWebhookJobTest.php | 31 ++- 12 files changed, 440 insertions(+), 85 deletions(-) create mode 100644 tests/Unit/NotificationWebhookSafetyTest.php diff --git a/app/Jobs/SendMessageToDiscordJob.php b/app/Jobs/SendMessageToDiscordJob.php index 99aeaeea2..9ac017396 100644 --- a/app/Jobs/SendMessageToDiscordJob.php +++ b/app/Jobs/SendMessageToDiscordJob.php @@ -3,6 +3,7 @@ namespace App\Jobs; use App\Notifications\Dto\DiscordMessage; +use App\Rules\SafeWebhookUrl; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; @@ -10,6 +11,8 @@ use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Validator; class SendMessageToDiscordJob implements ShouldBeEncrypted, ShouldQueue { @@ -41,6 +44,20 @@ public function __construct( */ public function handle(): void { - Http::post($this->webhookUrl, $this->message->toPayload()); + $validator = Validator::make( + ['webhook_url' => $this->webhookUrl], + ['webhook_url' => ['required', 'url', new SafeWebhookUrl]] + ); + + if ($validator->fails()) { + Log::warning('SendMessageToDiscordJob: blocked unsafe webhook URL', [ + 'url' => $this->webhookUrl, + 'errors' => $validator->errors()->all(), + ]); + + return; + } + + Http::withOptions(['allow_redirects' => false])->post($this->webhookUrl, $this->message->toPayload()); } } diff --git a/app/Jobs/SendMessageToSlackJob.php b/app/Jobs/SendMessageToSlackJob.php index f869fd602..e5cff5818 100644 --- a/app/Jobs/SendMessageToSlackJob.php +++ b/app/Jobs/SendMessageToSlackJob.php @@ -3,6 +3,7 @@ namespace App\Jobs; use App\Notifications\Dto\SlackMessage; +use App\Rules\SafeWebhookUrl; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; @@ -10,6 +11,8 @@ use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Validator; class SendMessageToSlackJob implements ShouldBeEncrypted, ShouldQueue { @@ -34,6 +37,20 @@ public function __construct( public function handle(): void { + $validator = Validator::make( + ['webhook_url' => $this->webhookUrl], + ['webhook_url' => ['required', 'url', new SafeWebhookUrl]] + ); + + if ($validator->fails()) { + Log::warning('SendMessageToSlackJob: blocked unsafe webhook URL', [ + 'url' => $this->webhookUrl, + 'errors' => $validator->errors()->all(), + ]); + + return; + } + if ($this->isSlackWebhook()) { $this->sendToSlack(); @@ -64,7 +81,7 @@ private function isSlackWebhook(): bool private function sendToSlack(): void { - Http::post($this->webhookUrl, [ + Http::withOptions(['allow_redirects' => false])->post($this->webhookUrl, [ 'text' => $this->message->title, 'blocks' => [ [ @@ -106,7 +123,7 @@ private function sendToMattermost(): void { $username = config('app.name'); - Http::post($this->webhookUrl, [ + Http::withOptions(['allow_redirects' => false])->post($this->webhookUrl, [ 'username' => $username, 'attachments' => [ [ diff --git a/app/Jobs/SendWebhookJob.php b/app/Jobs/SendWebhookJob.php index 17517cebb..beee24179 100644 --- a/app/Jobs/SendWebhookJob.php +++ b/app/Jobs/SendWebhookJob.php @@ -64,7 +64,7 @@ public function handle(): void ]); } - $response = Http::post($this->webhookUrl, $this->payload); + $response = Http::withOptions(['allow_redirects' => false])->post($this->webhookUrl, $this->payload); if (isDev()) { ray('Webhook response', [ diff --git a/app/Models/S3Storage.php b/app/Models/S3Storage.php index 3b344dfff..74edb5fa2 100644 --- a/app/Models/S3Storage.php +++ b/app/Models/S3Storage.php @@ -165,6 +165,7 @@ public function testConnection(bool $shouldSave = false) 'http' => [ 'connect_timeout' => self::CONNECTION_TIMEOUT_SECONDS, 'timeout' => self::REQUEST_TIMEOUT_SECONDS, + 'allow_redirects' => false, ], ]); // Test the connection by listing files with ListObjectsV2 (S3) diff --git a/app/Rules/SafeExternalUrl.php b/app/Rules/SafeExternalUrl.php index 41299d6c1..5380dd5e3 100644 --- a/app/Rules/SafeExternalUrl.php +++ b/app/Rules/SafeExternalUrl.php @@ -8,6 +8,11 @@ class SafeExternalUrl implements ValidationRule { + /** + * @param (Closure(string): array)|null $resolver + */ + public function __construct(private ?Closure $resolver = null) {} + /** * Run the validation rule. * @@ -38,44 +43,137 @@ public function validate(string $attribute, mixed $value, Closure $fail): void } $host = strtolower($host); + $hostForIpCheck = $this->normalizeHostForIpCheck($host); + $hostForDns = rtrim($hostForIpCheck, '.'); - // Block well-known internal hostnames $internalHosts = ['localhost', '0.0.0.0', '::1']; - if (in_array($host, $internalHosts) || str_ends_with($host, '.local') || str_ends_with($host, '.internal')) { - Log::warning('External URL points to internal host', [ - 'attribute' => $attribute, - 'url' => $value, - 'host' => $host, - 'ip' => request()->ip(), - 'user_id' => auth()->id(), - ]); + if (in_array($hostForDns, $internalHosts, true) || str_ends_with($hostForDns, '.local') || str_ends_with($hostForDns, '.internal')) { + $this->logBlockedHost($attribute, $value, $host); $fail('The :attribute must not point to internal hosts.'); return; } - // Resolve hostname to IP and block private/reserved ranges - $ip = gethostbyname($host); + if (filter_var($hostForIpCheck, FILTER_VALIDATE_IP)) { + if (! $this->isPublicIp($hostForIpCheck)) { + $this->logBlockedIp($attribute, $value, $host, $hostForIpCheck); + $fail('The :attribute must not point to a private or reserved IP address.'); - // gethostbyname returns the original hostname on failure (e.g. unresolvable) - if ($ip === $host && ! filter_var($host, FILTER_VALIDATE_IP)) { + return; + } + + return; + } + + $resolvedIps = $this->resolveHost($hostForDns); + if ($resolvedIps === []) { $fail('The :attribute host could not be resolved.'); return; } - if (! filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) { - Log::warning('External URL resolves to private or reserved IP', [ - 'attribute' => $attribute, - 'url' => $value, - 'host' => $host, - 'resolved_ip' => $ip, - 'ip' => request()->ip(), - 'user_id' => auth()->id(), - ]); - $fail('The :attribute must not point to a private or reserved IP address.'); + foreach ($resolvedIps as $resolvedIp) { + if (! $this->isPublicIp($resolvedIp)) { + $this->logBlockedIp($attribute, $value, $host, $resolvedIp); + $fail('The :attribute must not point to a private or reserved IP address.'); - return; + return; + } } } + + private function normalizeHostForIpCheck(string $host): string + { + return (str_starts_with($host, '[') && str_ends_with($host, ']')) + ? substr($host, 1, -1) + : $host; + } + + /** + * @return array + */ + private function resolveHost(string $host): array + { + if ($this->resolver instanceof Closure) { + return array_values(array_filter(($this->resolver)($host), fn (string $ip): bool => filter_var($ip, FILTER_VALIDATE_IP) !== false)); + } + + $records = @dns_get_record($host, DNS_A | DNS_AAAA); + if ($records === false) { + $records = []; + } + + $ips = []; + foreach ($records as $record) { + foreach (['ip', 'ipv6'] as $key) { + if (isset($record[$key]) && filter_var($record[$key], FILTER_VALIDATE_IP)) { + $ips[] = $record[$key]; + } + } + } + + $ipv4Addresses = @gethostbynamel($host); + if (is_array($ipv4Addresses)) { + foreach ($ipv4Addresses as $ip) { + if (filter_var($ip, FILTER_VALIDATE_IP)) { + $ips[] = $ip; + } + } + } + + return array_values(array_unique($ips)); + } + + private function isPublicIp(string $ip): bool + { + $embeddedIpv4 = $this->extractIpv4FromMappedIpv6($ip); + if ($embeddedIpv4 !== null) { + return filter_var($embeddedIpv4, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) !== false; + } + + return filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) !== false; + } + + private function extractIpv4FromMappedIpv6(string $ip): ?string + { + $packed = @inet_pton($ip); + if ($packed === false || strlen($packed) !== 16) { + return null; + } + + $prefix = substr($packed, 0, 12); + if ($prefix !== str_repeat("\0", 10)."\xff\xff") { + return null; + } + + $parts = unpack('C4', substr($packed, 12, 4)); + if ($parts === false) { + return null; + } + + return implode('.', $parts); + } + + private function logBlockedHost(string $attribute, string $url, string $host): void + { + Log::warning('External URL points to internal host', [ + 'attribute' => $attribute, + 'url' => $url, + 'host' => $host, + 'ip' => request()->ip(), + 'user_id' => auth()->id(), + ]); + } + + private function logBlockedIp(string $attribute, string $url, string $host, string $resolvedIp): void + { + Log::warning('External URL resolves to private or reserved IP', [ + 'attribute' => $attribute, + 'url' => $url, + 'host' => $host, + 'resolved_ip' => $resolvedIp, + 'ip' => request()->ip(), + 'user_id' => auth()->id(), + ]); + } } diff --git a/app/Rules/SafeWebhookUrl.php b/app/Rules/SafeWebhookUrl.php index 3723e1db5..ead03e9c0 100644 --- a/app/Rules/SafeWebhookUrl.php +++ b/app/Rules/SafeWebhookUrl.php @@ -8,6 +8,11 @@ class SafeWebhookUrl implements ValidationRule { + /** + * @param (Closure(string): array)|null $resolver + */ + public function __construct(private ?Closure $resolver = null) {} + /** * Run the validation rule. * @@ -39,63 +44,175 @@ public function validate(string $attribute, mixed $value, Closure $fail): void } $host = strtolower($host); + $hostForIpCheck = $this->normalizeHostForIpCheck($host); + $hostForDns = rtrim($hostForIpCheck, '.'); - // Strip IPv6 brackets (e.g. "[::1]" -> "::1") before IP checks so bracketed - // literals can't sneak past filter_var FILTER_VALIDATE_IP. - $hostForIpCheck = (str_starts_with($host, '[') && str_ends_with($host, ']')) - ? substr($host, 1, -1) - : $host; - - // Block well-known dangerous hostnames $blockedHosts = ['localhost', '0.0.0.0', '::1']; - if (in_array($hostForIpCheck, $blockedHosts) || str_ends_with($host, '.internal')) { - Log::warning('Webhook URL points to blocked host', [ - 'attribute' => $attribute, - 'host' => $host, - 'ip' => request()->ip(), - 'user_id' => auth()->id(), - ]); + if (in_array($hostForDns, $blockedHosts, true) || str_ends_with($hostForDns, '.internal')) { + $this->logBlockedHost($attribute, $host); $fail('The :attribute must not point to localhost or internal hosts.'); return; } - // Block loopback (127.0.0.0/8) and link-local/metadata (169.254.0.0/16) when IP is provided directly - if (filter_var($hostForIpCheck, FILTER_VALIDATE_IP) && ($this->isLoopback($hostForIpCheck) || $this->isLinkLocal($hostForIpCheck))) { - Log::warning('Webhook URL points to blocked IP range', [ - 'attribute' => $attribute, - 'host' => $host, - 'ip' => request()->ip(), - 'user_id' => auth()->id(), - ]); - $fail('The :attribute must not point to loopback or link-local addresses.'); + if (filter_var($hostForIpCheck, FILTER_VALIDATE_IP)) { + if ($this->isBlockedIp($hostForIpCheck)) { + $this->logBlockedIp($attribute, $host, $hostForIpCheck); + $fail('The :attribute must not point to loopback or link-local addresses.'); + + return; + } return; } + + $resolvedIps = $this->resolveHost($hostForDns); + foreach ($resolvedIps as $resolvedIp) { + if ($this->isBlockedIp($resolvedIp)) { + $this->logBlockedIp($attribute, $host, $resolvedIp); + $fail('The :attribute must not point to loopback or link-local addresses.'); + + return; + } + } } - private function isLoopback(string $ip): bool + private function normalizeHostForIpCheck(string $host): string + { + return (str_starts_with($host, '[') && str_ends_with($host, ']')) + ? substr($host, 1, -1) + : $host; + } + + /** + * @return array + */ + private function resolveHost(string $host): array + { + if ($this->resolver instanceof Closure) { + return array_values(array_filter(($this->resolver)($host), fn (string $ip): bool => filter_var($ip, FILTER_VALIDATE_IP) !== false)); + } + + $records = @dns_get_record($host, DNS_A | DNS_AAAA); + if ($records === false) { + $records = []; + } + + $ips = []; + foreach ($records as $record) { + foreach (['ip', 'ipv6'] as $key) { + if (isset($record[$key]) && filter_var($record[$key], FILTER_VALIDATE_IP)) { + $ips[] = $record[$key]; + } + } + } + + $ipv4Addresses = @gethostbynamel($host); + if (is_array($ipv4Addresses)) { + foreach ($ipv4Addresses as $ip) { + if (filter_var($ip, FILTER_VALIDATE_IP)) { + $ips[] = $ip; + } + } + } + + return array_values(array_unique($ips)); + } + + private function isBlockedIp(string $ip): bool + { + $embeddedIpv4 = $this->extractIpv4FromMappedIpv6($ip); + if ($embeddedIpv4 !== null) { + return $this->isBlockedIpv4($embeddedIpv4); + } + + if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) { + return $this->isBlockedIpv4($ip); + } + + return $this->isBlockedIpv6($ip); + } + + private function isBlockedIpv4(string $ip): bool { - // 127.0.0.0/8, 0.0.0.0 if ($ip === '0.0.0.0' || str_starts_with($ip, '127.')) { return true; } - // IPv6 loopback - $normalized = @inet_pton($ip); - - return $normalized !== false && $normalized === inet_pton('::1'); - } - - private function isLinkLocal(string $ip): bool - { - // 169.254.0.0/16 — covers cloud metadata at 169.254.169.254 - if (! filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) { + $long = ip2long($ip); + if ($long === false) { return false; } - $long = ip2long($ip); + $unsigned = sprintf('%u', $long); + $linkLocalStart = sprintf('%u', ip2long('169.254.0.0')); + $linkLocalEnd = sprintf('%u', ip2long('169.254.255.255')); - return $long !== false && ($long >> 16) === (ip2long('169.254.0.0') >> 16); + return $unsigned >= $linkLocalStart && $unsigned <= $linkLocalEnd; + } + + private function isBlockedIpv6(string $ip): bool + { + $packed = @inet_pton($ip); + if ($packed === false) { + return false; + } + + if ($packed === inet_pton('::1') || $packed === inet_pton('::')) { + return true; + } + + $bytes = unpack('C16', $packed); + if ($bytes === false) { + return false; + } + + $firstByte = $bytes[1]; + $secondByte = $bytes[2]; + + // fe80::/10 link-local and fc00::/7 unique local addresses. + return ($firstByte === 0xFE && ($secondByte & 0xC0) === 0x80) + || (($firstByte & 0xFE) === 0xFC); + } + + private function extractIpv4FromMappedIpv6(string $ip): ?string + { + $packed = @inet_pton($ip); + if ($packed === false || strlen($packed) !== 16) { + return null; + } + + $prefix = substr($packed, 0, 12); + if ($prefix !== str_repeat("\0", 10)."\xff\xff") { + return null; + } + + $parts = unpack('C4', substr($packed, 12, 4)); + if ($parts === false) { + return null; + } + + return implode('.', $parts); + } + + private function logBlockedHost(string $attribute, string $host): void + { + Log::warning('Webhook URL points to blocked host', [ + 'attribute' => $attribute, + 'host' => $host, + 'ip' => request()->ip(), + 'user_id' => auth()->id(), + ]); + } + + private function logBlockedIp(string $attribute, string $host, string $blockedIp): void + { + Log::warning('Webhook URL points to blocked IP range', [ + 'attribute' => $attribute, + 'host' => $host, + 'resolved_ip' => $blockedIp, + 'ip' => request()->ip(), + 'user_id' => auth()->id(), + ]); } } diff --git a/tests/Unit/NotificationWebhookSafetyTest.php b/tests/Unit/NotificationWebhookSafetyTest.php new file mode 100644 index 000000000..9246c4004 --- /dev/null +++ b/tests/Unit/NotificationWebhookSafetyTest.php @@ -0,0 +1,36 @@ +handle(); + + Http::assertNothingSent(); +}); + +it('blocks queued Discord notifications to IPv4-mapped link-local URLs', function () { + Http::fake(); + + $job = new SendMessageToDiscordJob( + new DiscordMessage('Test', 'Description', DiscordMessage::infoColor()), + 'http://[::ffff:169.254.169.254]/' + ); + + $job->handle(); + + Http::assertNothingSent(); +}); diff --git a/tests/Unit/S3StorageEndpointValidationTest.php b/tests/Unit/S3StorageEndpointValidationTest.php index 054606a25..bfc2fd18b 100644 --- a/tests/Unit/S3StorageEndpointValidationTest.php +++ b/tests/Unit/S3StorageEndpointValidationTest.php @@ -23,8 +23,9 @@ expect($validator->fails())->toBeTrue("Expected rejection: {$endpoint}"); })->with([ - 'AWS IMDS' => 'http://169.254.169.254/latest/meta-data/', - 'AWS IMDS bare' => 'http://169.254.169.254', + 'link-local address' => 'http://169.254.169.254/', + 'link-local address bare' => 'http://169.254.169.254', + 'link-local address IPv4-mapped IPv6' => 'http://[::ffff:169.254.169.254]/', 'GCP metadata via link-local' => 'http://169.254.0.1', 'loopback v4' => 'http://127.0.0.1', 'loopback Redis' => 'http://127.0.0.1:6379', @@ -87,5 +88,6 @@ 'http loopback' => 'http://127.0.0.1:6379', 'localhost' => 'http://localhost:9000', 'IPv6 loopback' => 'http://[::1]', + 'IPv4-mapped IPv6 link-local' => 'http://[::ffff:169.254.169.254]', 'internal TLD' => 'http://backend.internal', ]); diff --git a/tests/Unit/S3StorageTest.php b/tests/Unit/S3StorageTest.php index ddf390443..4cf56640e 100644 --- a/tests/Unit/S3StorageTest.php +++ b/tests/Unit/S3StorageTest.php @@ -53,6 +53,7 @@ $s3Storage = new S3Storage; expect($s3Storage->getFillable())->toBe([ + 'team_id', 'name', 'description', 'region', @@ -74,6 +75,7 @@ ->with(Mockery::on(function (array $config) { expect($config['http']['connect_timeout'])->toBe(15); expect($config['http']['timeout'])->toBe(15); + expect($config['http']['allow_redirects'])->toBeFalse(); return true; })) diff --git a/tests/Unit/SafeExternalUrlTest.php b/tests/Unit/SafeExternalUrlTest.php index b2bc13337..c047b1923 100644 --- a/tests/Unit/SafeExternalUrlTest.php +++ b/tests/Unit/SafeExternalUrlTest.php @@ -11,7 +11,7 @@ $validUrls = [ 'https://api.github.com', - 'https://github.example.com/api/v3', + 'https://github.com/api/v3', 'https://example.com', 'http://example.com', ]; @@ -22,6 +22,14 @@ } }); +it('accepts custom external hostnames that resolve to public IPs', function () { + $rule = new SafeExternalUrl(fn (string $host): array => ['93.184.216.34']); + + $validator = Validator::make(['url' => 'https://github.example.com/api/v3'], ['url' => $rule]); + + expect($validator->passes())->toBeTrue('Expected valid custom external hostname'); +}); + it('rejects private IPv4 addresses', function (string $url) { $rule = new SafeExternalUrl; @@ -42,6 +50,34 @@ expect($validator->fails())->toBeTrue('Expected rejection: cloud metadata IP'); }); +it('rejects hostnames that resolve to private or reserved addresses', function (string $url, array $resolvedIps) { + $rule = new SafeExternalUrl(fn (string $host): array => $resolvedIps); + + $validator = Validator::make(['url' => $url], ['url' => $rule]); + + expect($validator->fails())->toBeTrue("Expected rejection after DNS resolution: {$url}"); +})->with([ + 'hostname to link-local IP' => ['http://169.254.169.254.nip.io/', ['169.254.169.254']], + 'hostname to loopback' => ['http://loopback.example.test/', ['127.0.0.1']], + 'hostname to private IPv4' => ['http://private.example.test/', ['10.0.0.1']], + 'hostname to IPv6 loopback' => ['http://ipv6-loopback.example.test/', ['::1']], + 'hostname to IPv6 link-local' => ['http://ipv6-link-local.example.test/', ['fe80::1']], + 'hostname to IPv6 ULA' => ['http://ipv6-ula.example.test/', ['fc00::1']], + 'hostname to mapped private IPv4' => ['http://mapped-private.example.test/', ['::ffff:10.0.0.1']], +]); + +it('rejects IPv4-mapped IPv6 literals for private or reserved IPv4 ranges', function (string $url) { + $rule = new SafeExternalUrl; + + $validator = Validator::make(['url' => $url], ['url' => $rule]); + + expect($validator->fails())->toBeTrue("Expected rejection: {$url}"); +})->with([ + 'mapped link-local IP' => 'http://[::ffff:169.254.169.254]/', + 'mapped loopback' => 'http://[::ffff:127.0.0.1]/', + 'mapped private' => 'http://[::ffff:10.0.0.1]/', +]); + it('rejects localhost and internal hostnames', function (string $url) { $rule = new SafeExternalUrl; @@ -50,9 +86,12 @@ })->with([ 'localhost' => 'http://localhost', 'localhost with port' => 'http://localhost:8080', + 'localhost with trailing dot' => 'http://localhost.', 'zero address' => 'http://0.0.0.0', '.local domain' => 'http://myservice.local', + '.local domain with trailing dot' => 'http://myservice.local.', '.internal domain' => 'http://myservice.internal', + '.internal domain with trailing dot' => 'http://myservice.internal.', ]); it('rejects non-URL strings', function (string $value) { diff --git a/tests/Unit/SafeWebhookUrlTest.php b/tests/Unit/SafeWebhookUrlTest.php index bb5569ccf..69dc6fbb5 100644 --- a/tests/Unit/SafeWebhookUrlTest.php +++ b/tests/Unit/SafeWebhookUrlTest.php @@ -59,6 +59,33 @@ expect($validator->fails())->toBeTrue('Expected rejection: link-local IP'); }); +it('rejects hostnames that resolve to blocked addresses', function (string $url, array $resolvedIps) { + $rule = new SafeWebhookUrl(fn (string $host): array => $resolvedIps); + + $validator = Validator::make(['url' => $url], ['url' => $rule]); + + expect($validator->fails())->toBeTrue("Expected rejection after DNS resolution: {$url}"); +})->with([ + 'hostname to link-local IP' => ['http://169.254.169.254.nip.io/', ['169.254.169.254']], + 'hostname to loopback' => ['http://loopback.example.test/', ['127.0.0.1']], + 'hostname to IPv6 loopback' => ['http://ipv6-loopback.example.test/', ['::1']], + 'hostname to IPv6 link-local' => ['http://ipv6-link-local.example.test/', ['fe80::1']], + 'hostname to IPv6 ULA' => ['http://ipv6-ula.example.test/', ['fc00::1']], + 'hostname to mapped link-local IP' => ['http://mapped-link-local.example.test/', ['::ffff:169.254.169.254']], +]); + +it('rejects IPv4-mapped IPv6 literals for blocked IPv4 ranges', function (string $url) { + $rule = new SafeWebhookUrl; + + $validator = Validator::make(['url' => $url], ['url' => $rule]); + + expect($validator->fails())->toBeTrue("Expected rejection: {$url}"); +})->with([ + 'mapped link-local IP' => 'http://[::ffff:169.254.169.254]/', + 'mapped loopback' => 'http://[::ffff:127.0.0.1]/', + 'mapped zero' => 'http://[::ffff:0.0.0.0]/', +]); + it('rejects localhost and internal hostnames', function (string $url) { $rule = new SafeWebhookUrl; @@ -67,7 +94,9 @@ })->with([ 'localhost' => 'http://localhost', 'localhost with port' => 'http://localhost:8080', + 'localhost with trailing dot' => 'http://localhost.', '.internal domain' => 'http://myservice.internal', + '.internal domain with trailing dot' => 'http://myservice.internal.', ]); it('rejects non-http schemes', function (string $value) { diff --git a/tests/Unit/SendWebhookJobTest.php b/tests/Unit/SendWebhookJobTest.php index 688cd3bf2..dedf18e1f 100644 --- a/tests/Unit/SendWebhookJobTest.php +++ b/tests/Unit/SendWebhookJobTest.php @@ -2,7 +2,6 @@ use App\Jobs\SendWebhookJob; use Illuminate\Support\Facades\Http; -use Illuminate\Support\Facades\Log; use Tests\TestCase; uses(TestCase::class); @@ -24,11 +23,6 @@ it('blocks webhook to loopback address', function () { Http::fake(); - Log::shouldReceive('warning') - ->once() - ->withArgs(function ($message) { - return str_contains($message, 'blocked unsafe webhook URL'); - }); $job = new SendWebhookJob( payload: ['event' => 'test'], @@ -42,15 +36,23 @@ it('blocks webhook to cloud metadata endpoint', function () { Http::fake(); - Log::shouldReceive('warning') - ->once() - ->withArgs(function ($message) { - return str_contains($message, 'blocked unsafe webhook URL'); - }); $job = new SendWebhookJob( payload: ['event' => 'test'], - webhookUrl: 'http://169.254.169.254/latest/meta-data/' + webhookUrl: 'http://169.254.169.254/' + ); + + $job->handle(); + + Http::assertNothingSent(); +}); + +it('blocks webhook to IPv4-mapped IPv6 link-local endpoint', function () { + Http::fake(); + + $job = new SendWebhookJob( + payload: ['event' => 'test'], + webhookUrl: 'http://[::ffff:169.254.169.254]/' ); $job->handle(); @@ -60,11 +62,6 @@ it('blocks webhook to localhost', function () { Http::fake(); - Log::shouldReceive('warning') - ->once() - ->withArgs(function ($message) { - return str_contains($message, 'blocked unsafe webhook URL'); - }); $job = new SendWebhookJob( payload: ['event' => 'test'],