diff --git a/app/Http/Controllers/SecurePathController.php b/app/Http/Controllers/SecurePathController.php index bb994d21d0c..16f8e40785f 100644 --- a/app/Http/Controllers/SecurePathController.php +++ b/app/Http/Controllers/SecurePathController.php @@ -33,25 +33,19 @@ class SecurePathController extends Controller public function __invoke(Request $request, ?string $path) { - // In theory we should use the `$request->hasCorrectSignature()` method here. - // However, for some stupid unknown reason, the path value is added to the server Query String. - // This completely invalidates the signature check. - // For example the url http://localhost:8000/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png?expires=1748380289 - // will be verified as : - // http://localhost:8000/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png?/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png&expires=1748380289 - // which makes the signature check fail as the hmac does not match. - if (!self::shouldNotUseSignedUrl() && !$this->hasCorrectSignature($request)) { + // First we verify that the request has not expired. + if (!self::shouldNotUseSignedUrl() && !$this->signatureHasNotExpired($request)) { + throw new SignatureExpiredException(); + } + + // Then we verify that the request has a valid signature. + if (!self::shouldNotUseSignedUrl() && !$request->hasValidSignature()) { Log::error('Invalid signature for secure path request. Verify that the url generated for the image match.', [ 'candidate url' => $this->getUrl($request), ]); throw new InvalidSignatureException(); } - // On the bright side, we can now differentiate between a missing/failed signature and an expired one. - if (!self::shouldNotUseSignedUrl() && !$this->signatureHasNotExpired($request)) { - throw new SignatureExpiredException(); - } - if (is_null($path)) { throw new WrongPathException(); } @@ -72,41 +66,16 @@ public function __invoke(Request $request, ?string $path) return response()->file($file); } - private function getUrl(Request $request, bool $absolute = true): string + private function getUrl(Request $request): string { $ignore_query = ['signature']; - $url = $absolute ? $request->url() : ('/' . $request->path()); - $query_string = ''; $query_string = (new Collection(explode('&', (string) $request->server->get('QUERY_STRING')))) ->reject(fn ($parameter) => in_array(\Str::before($parameter, '='), $ignore_query, true)) - ->reject(fn ($parameter) => count(explode('=', $parameter)) === 1) // Ignore parameters without value => avoid problem above mentionned. ->join('&'); - return rtrim($url . '?' . $query_string, '?'); - } - - /** - * Determine if the signature from the given request matches the URL. - * - * @param \Illuminate\Http\Request $request - * @param bool $absolute - * - * @return bool - */ - private function hasCorrectSignature(Request $request, bool $absolute = true): bool - { - $original = $this->getUrl($request, $absolute); - $key = new \SensitiveParameterValue(config('app.key')); - if (hash_equals( - hash_hmac('sha256', $original, $key->getValue()), - $request->query('signature', '') - )) { - return true; - } - - return false; + return rtrim($request->url() . '?' . $query_string, '?'); } /**