refactor(storage): tighten S3 endpoint URL validation (#9668)
This commit is contained in:
commit
03a35faf2c
5 changed files with 113 additions and 6 deletions
|
|
@ -3,6 +3,7 @@
|
|||
namespace App\Livewire\Storage;
|
||||
|
||||
use App\Models\S3Storage;
|
||||
use App\Rules\SafeWebhookUrl;
|
||||
use App\Support\ValidationPatterns;
|
||||
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
|
||||
use Illuminate\Support\Uri;
|
||||
|
|
@ -37,7 +38,7 @@ protected function rules(): array
|
|||
'key' => 'required|max:255',
|
||||
'secret' => 'required|max:255',
|
||||
'bucket' => 'required|max:255',
|
||||
'endpoint' => 'required|url|max:255',
|
||||
'endpoint' => ['required', 'max:255', new SafeWebhookUrl],
|
||||
];
|
||||
}
|
||||
|
||||
|
|
@ -55,7 +56,6 @@ protected function messages(): array
|
|||
'bucket.required' => 'The Bucket field is required.',
|
||||
'bucket.max' => 'The Bucket may not be greater than 255 characters.',
|
||||
'endpoint.required' => 'The Endpoint field is required.',
|
||||
'endpoint.url' => 'The Endpoint must be a valid URL.',
|
||||
'endpoint.max' => 'The Endpoint may not be greater than 255 characters.',
|
||||
]
|
||||
);
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
namespace App\Livewire\Storage;
|
||||
|
||||
use App\Models\S3Storage;
|
||||
use App\Rules\SafeWebhookUrl;
|
||||
use App\Support\ValidationPatterns;
|
||||
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
|
|
@ -42,7 +43,7 @@ protected function rules(): array
|
|||
'key' => 'required|max:255',
|
||||
'secret' => 'required|max:255',
|
||||
'bucket' => 'required|max:255',
|
||||
'endpoint' => 'required|url|max:255',
|
||||
'endpoint' => ['required', 'max:255', new SafeWebhookUrl],
|
||||
];
|
||||
}
|
||||
|
||||
|
|
@ -60,7 +61,6 @@ protected function messages(): array
|
|||
'bucket.required' => 'The Bucket field is required.',
|
||||
'bucket.max' => 'The Bucket may not be greater than 255 characters.',
|
||||
'endpoint.required' => 'The Endpoint field is required.',
|
||||
'endpoint.url' => 'The Endpoint must be a valid URL.',
|
||||
'endpoint.max' => 'The Endpoint may not be greater than 255 characters.',
|
||||
]
|
||||
);
|
||||
|
|
|
|||
|
|
@ -2,11 +2,13 @@
|
|||
|
||||
namespace App\Models;
|
||||
|
||||
use App\Rules\SafeWebhookUrl;
|
||||
use App\Traits\HasSafeStringAttribute;
|
||||
use Illuminate\Database\Eloquent\Casts\Attribute;
|
||||
use Illuminate\Database\Eloquent\Factories\HasFactory;
|
||||
use Illuminate\Notifications\Messages\MailMessage;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Illuminate\Support\Facades\Validator;
|
||||
|
||||
class S3Storage extends BaseModel
|
||||
{
|
||||
|
|
@ -139,6 +141,14 @@ protected function region(): Attribute
|
|||
public function testConnection(bool $shouldSave = false)
|
||||
{
|
||||
try {
|
||||
$validator = Validator::make(
|
||||
['endpoint' => $this['endpoint']],
|
||||
['endpoint' => ['required', new SafeWebhookUrl]],
|
||||
);
|
||||
if ($validator->fails()) {
|
||||
throw new \RuntimeException('S3 endpoint is not allowed: '.$validator->errors()->first('endpoint'));
|
||||
}
|
||||
|
||||
$disk = Storage::build([
|
||||
'driver' => 's3',
|
||||
'region' => $this['region'],
|
||||
|
|
|
|||
|
|
@ -40,9 +40,15 @@ public function validate(string $attribute, mixed $value, Closure $fail): void
|
|||
|
||||
$host = strtolower($host);
|
||||
|
||||
// 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($host, $blockedHosts) || str_ends_with($host, '.internal')) {
|
||||
if (in_array($hostForIpCheck, $blockedHosts) || str_ends_with($host, '.internal')) {
|
||||
Log::warning('Webhook URL points to blocked host', [
|
||||
'attribute' => $attribute,
|
||||
'host' => $host,
|
||||
|
|
@ -55,7 +61,7 @@ public function validate(string $attribute, mixed $value, Closure $fail): void
|
|||
}
|
||||
|
||||
// Block loopback (127.0.0.0/8) and link-local/metadata (169.254.0.0/16) when IP is provided directly
|
||||
if (filter_var($host, FILTER_VALIDATE_IP) && ($this->isLoopback($host) || $this->isLinkLocal($host))) {
|
||||
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,
|
||||
|
|
|
|||
91
tests/Unit/S3StorageEndpointValidationTest.php
Normal file
91
tests/Unit/S3StorageEndpointValidationTest.php
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
<?php
|
||||
|
||||
use App\Models\S3Storage;
|
||||
use App\Rules\SafeWebhookUrl;
|
||||
use Illuminate\Support\Facades\Validator;
|
||||
use Tests\TestCase;
|
||||
|
||||
uses(TestCase::class);
|
||||
|
||||
/**
|
||||
* Regression tests for GHSA-pwm4-w33c-wjf3 — SSRF via S3 Storage endpoint.
|
||||
*
|
||||
* The Livewire forms (Create.php, Form.php) and the model-level defense in
|
||||
* S3Storage::testConnection() share the same SafeWebhookUrl rule. These tests
|
||||
* assert the rule rejects the concrete payloads from the advisory PoC and
|
||||
* that the model refuses to build an S3 client for an unsafe endpoint.
|
||||
*/
|
||||
it('rejects SSRF payloads from the GHSA-pwm4-w33c-wjf3 advisory', function (string $endpoint) {
|
||||
$validator = Validator::make(
|
||||
['endpoint' => $endpoint],
|
||||
['endpoint' => ['required', 'max:255', new SafeWebhookUrl]],
|
||||
);
|
||||
|
||||
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',
|
||||
'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',
|
||||
'loopback Postgres' => 'http://127.0.0.1:5432',
|
||||
'loopback alt in /8' => 'http://127.10.20.30',
|
||||
'zero address' => 'http://0.0.0.0',
|
||||
'IPv6 loopback' => 'http://[::1]',
|
||||
'localhost hostname' => 'http://localhost',
|
||||
'localhost with port' => 'http://localhost:9000',
|
||||
'internal suffix' => 'http://minio.internal',
|
||||
'file scheme' => 'file:///etc/passwd',
|
||||
'javascript scheme' => 'javascript:alert(1)',
|
||||
]);
|
||||
|
||||
it('accepts real-world S3 endpoints', function (string $endpoint) {
|
||||
$validator = Validator::make(
|
||||
['endpoint' => $endpoint],
|
||||
['endpoint' => ['required', 'max:255', new SafeWebhookUrl]],
|
||||
);
|
||||
|
||||
expect($validator->passes())->toBeTrue("Expected accepted: {$endpoint}");
|
||||
})->with([
|
||||
'AWS S3' => 'https://s3.us-east-1.amazonaws.com',
|
||||
'Cloudflare R2' => 'https://fake.r2.cloudflarestorage.com',
|
||||
'DigitalOcean Spaces' => 'https://nyc3.digitaloceanspaces.com',
|
||||
'Backblaze B2' => 'https://s3.us-west-001.backblazeb2.com',
|
||||
'Self-hosted MinIO on 10.x' => 'http://10.0.0.5:9000',
|
||||
'Self-hosted MinIO on 172.16.x' => 'http://172.16.0.10:9000',
|
||||
'Self-hosted MinIO on 192.168.x' => 'http://192.168.1.50:9000',
|
||||
'Custom domain MinIO' => 'https://minio.example.com',
|
||||
]);
|
||||
|
||||
it('blocks testConnection() on an unsafe endpoint without issuing HTTP', function () {
|
||||
$s3Storage = new S3Storage;
|
||||
$s3Storage->setRawAttributes([
|
||||
'region' => 'us-east-1',
|
||||
'key' => 'AKIAEXAMPLE',
|
||||
'secret' => 'secret',
|
||||
'bucket' => 'latest/meta-data',
|
||||
'endpoint' => 'http://169.254.169.254',
|
||||
]);
|
||||
|
||||
expect(fn () => $s3Storage->testConnection())
|
||||
->toThrow(RuntimeException::class, 'S3 endpoint is not allowed');
|
||||
});
|
||||
|
||||
it('blocks testConnection() for loopback endpoints', function (string $endpoint) {
|
||||
$s3Storage = new S3Storage;
|
||||
$s3Storage->setRawAttributes([
|
||||
'region' => 'us-east-1',
|
||||
'key' => 'AKIAEXAMPLE',
|
||||
'secret' => 'secret',
|
||||
'bucket' => 'bucket',
|
||||
'endpoint' => $endpoint,
|
||||
]);
|
||||
|
||||
expect(fn () => $s3Storage->testConnection())
|
||||
->toThrow(RuntimeException::class, 'S3 endpoint is not allowed');
|
||||
})->with([
|
||||
'http loopback' => 'http://127.0.0.1:6379',
|
||||
'localhost' => 'http://localhost:9000',
|
||||
'IPv6 loopback' => 'http://[::1]',
|
||||
'internal TLD' => 'http://backend.internal',
|
||||
]);
|
||||
Loading…
Reference in a new issue