From 9e84ad261d375cf8761615f2518dc79d25e6d511 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 9 Sep 2026 19:44:01 +0930 Subject: [PATCH] Route StoryFetch outbound requests through SSRF-hardened fetch service --- app/Jobs/StoryPipeline/StoryFetch.php | 107 +++--------------- app/Services/SecureMediaFetchService.php | 22 +++- tests/Feature/SecureMediaFetchHeadersTest.php | 99 ++++++++++++++++ tests/Feature/StoryFetchSsrfTest.php | 74 ++++++++++++ 4 files changed, 207 insertions(+), 95 deletions(-) create mode 100644 tests/Feature/SecureMediaFetchHeadersTest.php create mode 100644 tests/Feature/StoryFetchSsrfTest.php diff --git a/app/Jobs/StoryPipeline/StoryFetch.php b/app/Jobs/StoryPipeline/StoryFetch.php index ffc00fbbf..fcce3eef0 100644 --- a/app/Jobs/StoryPipeline/StoryFetch.php +++ b/app/Jobs/StoryPipeline/StoryFetch.php @@ -4,6 +4,7 @@ namespace App\Jobs\StoryPipeline; use App\Models\Story; use App\Services\MediaPathService; +use App\Services\SecureMediaFetchService; use App\Services\StoryIndexService; use App\Services\StoryService; use App\Util\ActivityPub\Helpers; @@ -13,14 +14,11 @@ use Exception; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; -use Illuminate\Http\Client\ConnectionException; -use Illuminate\Http\Client\RequestException; use Illuminate\Http\File; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Facades\Validator; @@ -34,10 +32,6 @@ class StoryFetch implements ShouldQueue private const MAX_DURATION = 300; - private const REQUEST_TIMEOUT = 30; - - private const MAX_REDIRECTS = 3; - // Rate limiting public $tries = 3; @@ -281,28 +275,21 @@ class StoryFetch implements ShouldQueue ]; try { - $response = Http::withHeaders($headers) - ->timeout(self::REQUEST_TIMEOUT) - ->connectTimeout(10) - ->retry(2, 1000) - ->withOptions([ - 'verify' => true, - 'max_redirects' => self::MAX_REDIRECTS, - ]) - ->get($url); - - if (! $response->successful()) { + // Fetch the bearcap story JSON through the SSRF-hardened path so the + // request cannot be redirected to an internal address. The bearer + // token is forwarded only to the original host and stripped on any + // cross-origin redirect hop (see SecureMediaFetchService::request). + $body = SecureMediaFetchService::get($url, null, null, $headers); + + if ($body === false) { if (config('app.dev_log')) { - Log::warning('Story fetch failed', [ - 'url' => $url, - 'status' => $response->status(), - ]); + Log::warning('Story fetch failed', ['url' => $url]); } return null; } - $payload = $response->json(); + $payload = json_decode($body, true); if (! is_array($payload)) { if (config('app.dev_log')) { @@ -314,15 +301,6 @@ class StoryFetch implements ShouldQueue return $payload; - } catch (RequestException|ConnectionException $e) { - if (config('app.dev_log')) { - Log::warning('HTTP request failed', [ - 'url' => $url, - 'error' => $e->getMessage(), - ]); - } - - return null; } catch (Exception $e) { if (config('app.dev_log')) { Log::error('Unexpected error in story fetch', [ @@ -464,23 +442,12 @@ class StoryFetch implements ShouldQueue } try { - $contextOptions = [ - 'ssl' => [ - 'verify_peer' => true, - 'verify_peername' => true, - 'allow_self_signed' => false, - 'SNI_enabled' => true, - ], - 'http' => [ - 'timeout' => self::REQUEST_TIMEOUT, - 'max_redirects' => self::MAX_REDIRECTS, - 'user_agent' => 'Pixelfed/'.config('pixelfed.version'), - ], - ]; - - $ctx = stream_context_create($contextOptions); - - $data = $this->downloadWithSizeLimit($mediaUrl, $ctx); + // Fetch through the SSRF-hardened path: https-only, resolves + pins + // to public IPs (CURLOPT_RESOLVE), disables auto-redirects and + // re-validates every hop against private/reserved ranges, and caps + // the body size. Replaces the bare fopen() stream that followed + // redirects to arbitrary internal addresses without re-validation. + $data = SecureMediaFetchService::get($mediaUrl, $this->getMaxFileSizeBytes()); if (! $data) { return null; } @@ -533,48 +500,6 @@ class StoryFetch implements ShouldQueue } } - /** - * Download with size limit enforcement - */ - private function downloadWithSizeLimit(string $url, $context): ?string - { - $maxFileSizeBytes = $this->getMaxFileSizeBytes(); - - $handle = fopen($url, 'r', false, $context); - if (! $handle) { - if (config('app.dev_log')) { - Log::warning('Failed to open URL stream', ['url' => $url]); - } - - return null; - } - - $data = ''; - $size = 0; - - while (! feof($handle) && $size < $maxFileSizeBytes) { - $chunk = fread($handle, 8192); - if ($chunk === false) { - break; - } - - $data .= $chunk; - $size += strlen($chunk); - } - - fclose($handle); - - if ($size >= $maxFileSizeBytes) { - if (config('app.dev_log')) { - Log::warning('File too large', ['size' => $size, 'limit' => $maxFileSizeBytes]); - } - - return null; - } - - return $data; - } - /** * Validate downloaded file */ diff --git a/app/Services/SecureMediaFetchService.php b/app/Services/SecureMediaFetchService.php index 88d560c45..416004699 100644 --- a/app/Services/SecureMediaFetchService.php +++ b/app/Services/SecureMediaFetchService.php @@ -50,10 +50,10 @@ class SecureMediaFetchService * * @return string|false */ - public static function get(string $url, ?int $maxBytes = null, ?int $expectedLength = null) + public static function get(string $url, ?int $maxBytes = null, ?int $expectedLength = null, array $headers = []) { $maxBytes = $maxBytes ?? self::defaultMaxBytes(); - $result = (new self)->request($url, 'get', $maxBytes, $expectedLength); + $result = (new self)->request($url, 'get', $maxBytes, $expectedLength, $headers); if (! is_array($result)) { return false; @@ -69,10 +69,17 @@ class SecureMediaFetchService * @return array|false For 'head': ['length'=>int,'mime'=>string]. * For 'get': ['body'=>string,'length'=>int,'mime'=>string]. */ - protected function request(string $url, string $method, int $maxBytes, ?int $expectedLength = null) + protected function request(string $url, string $method, int $maxBytes, ?int $expectedLength = null, array $extraHeaders = []) { $currentUrl = $url; + // Host of the original request. Caller-supplied headers (e.g. an + // Authorization bearer token) are only sent to this host and are + // stripped on any cross-origin redirect hop, mirroring Guzzle's + // RedirectMiddleware credential-stripping behaviour. + $originHost = parse_url($url, PHP_URL_HOST); + $originHost = is_string($originHost) ? strtolower($originHost) : null; + for ($redirects = 0; $redirects <= self::MAX_REDIRECTS; $redirects++) { $currentUrl = Helpers::validateUrl($currentUrl); @@ -88,6 +95,13 @@ class SecureMediaFetchService return false; } + // Only forward caller headers when the current hop is the same host + // as the original request; drop them across origins. + $headers = ['User-Agent' => self::userAgent()]; + if (! empty($extraHeaders) && strtolower((string) $host) === $originHost) { + $headers = array_merge($extraHeaders, $headers); + } + // Resolve the host and reject if ANY resolved address is // non-global. Fail-closed: empty means unresolved or private. $ips = Helpers::resolvePublicIps($host); @@ -116,7 +130,7 @@ class SecureMediaFetchService } }, ]) - ->withHeaders(['User-Agent' => self::userAgent()]) + ->withHeaders($headers) ->timeout(self::TIMEOUT) ->connectTimeout(self::CONNECT_TIMEOUT) ->{$method}($currentUrl); diff --git a/tests/Feature/SecureMediaFetchHeadersTest.php b/tests/Feature/SecureMediaFetchHeadersTest.php new file mode 100644 index 000000000..7d7bae1d9 --- /dev/null +++ b/tests/Feature/SecureMediaFetchHeadersTest.php @@ -0,0 +1,99 @@ + Http::response('{"ok":true}', 200, [ + 'Content-Type' => 'application/json', + ]), + ]); + + $body = SecureMediaFetchService::get('https://origin.example/story', null, null, [ + 'Authorization' => 'Bearer secret-token', + ]); + + expect($body)->toBe('{"ok":true}'); + + Http::assertSent(function ($request) { + return $request->url() === 'https://origin.example/story' + && $request->hasHeader('Authorization', 'Bearer secret-token'); + }); +}); + +it('strips the Authorization header on a cross-origin redirect', function () { + seedPublicIp('origin.example'); + seedPublicIp('other.example'); + + Http::fake([ + 'https://origin.example/story' => Http::response('', 302, [ + 'Location' => 'https://other.example/story', + ]), + 'https://other.example/story' => Http::response('{"ok":true}', 200, [ + 'Content-Type' => 'application/json', + ]), + ]); + + $body = SecureMediaFetchService::get('https://origin.example/story', null, null, [ + 'Authorization' => 'Bearer secret-token', + ]); + + expect($body)->toBe('{"ok":true}'); + + // The cross-origin hop must NOT carry the bearer token. + Http::assertSent(function ($request) { + if ($request->url() !== 'https://other.example/story') { + return false; + } + + return ! $request->hasHeader('Authorization'); + }); +}); + +it('refuses to follow a redirect to a private address', function () { + seedPublicIp('origin.example'); + + Http::fake([ + 'https://origin.example/story' => Http::response('', 302, [ + 'Location' => 'http://169.254.169.254/latest/meta-data/', + ]), + // If the service (incorrectly) followed, this would answer; it must not. + '169.254.169.254/*' => Http::response('SECRET', 200), + ]); + + $body = SecureMediaFetchService::get('https://origin.example/story'); + + expect($body)->toBeFalse(); + + Http::assertNotSent(function ($request) { + return str_contains($request->url(), '169.254.169.254'); + }); +}); diff --git a/tests/Feature/StoryFetchSsrfTest.php b/tests/Feature/StoryFetchSsrfTest.php new file mode 100644 index 000000000..2be4aa0a3 --- /dev/null +++ b/tests/Feature/StoryFetchSsrfTest.php @@ -0,0 +1,74 @@ +setAccessible(true); + + return $ref->invoke($job, $url, $token); +} + +beforeEach(function () { + Cache::flush(); +}); + +it('fetches the story payload over the hardened path', function () { + seedPublicIpForStory('peer.example'); + + Http::fake([ + 'https://peer.example/story' => Http::response('{"id":"https://peer.example/s/1"}', 200, [ + 'Content-Type' => 'application/json', + ]), + ]); + + $payload = callFetchStoryPayload('https://peer.example/story', 'bearcap-token-1234567890'); + + expect($payload)->toBeArray() + ->and($payload['id'])->toBe('https://peer.example/s/1'); +}); + +it('refuses a payload fetch that redirects to a private address', function () { + seedPublicIpForStory('peer.example'); + + Http::fake([ + 'https://peer.example/story' => Http::response('', 302, [ + 'Location' => 'http://169.254.169.254/latest/meta-data/', + ]), + '169.254.169.254/*' => Http::response('SECRET', 200), + ]); + + $payload = callFetchStoryPayload('https://peer.example/story', 'bearcap-token-1234567890'); + + expect($payload)->toBeNull(); + + Http::assertNotSent(function ($request) { + return str_contains($request->url(), '169.254.169.254'); + }); +});