From dbda2bce2356bd6511d2c43109d62f3c1552fd9e Mon Sep 17 00:00:00 2001 From: ildyria Date: Tue, 27 May 2025 20:32:51 +0200 Subject: [PATCH 1/3] maybe? --- app/Http/Controllers/SecurePathController.php | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/app/Http/Controllers/SecurePathController.php b/app/Http/Controllers/SecurePathController.php index bb994d21d0c..dfea7f29f8d 100644 --- a/app/Http/Controllers/SecurePathController.php +++ b/app/Http/Controllers/SecurePathController.php @@ -40,18 +40,18 @@ public function __invoke(Request $request, ?string $path) // 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)) { + // 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 (!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(); } @@ -87,27 +87,27 @@ private function getUrl(Request $request, bool $absolute = true): string 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; - } + // /** + // * 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; + // } /** * Determine if the expires timestamp from the given request is not from the past. From 54e88e1996d7a85a268613ad1de0960856be572d Mon Sep 17 00:00:00 2001 From: ildyria Date: Tue, 27 May 2025 20:34:58 +0200 Subject: [PATCH 2/3] remove unnecessary comment --- app/Http/Controllers/SecurePathController.php | 32 ++----------------- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/app/Http/Controllers/SecurePathController.php b/app/Http/Controllers/SecurePathController.php index dfea7f29f8d..0599e04d093 100644 --- a/app/Http/Controllers/SecurePathController.php +++ b/app/Http/Controllers/SecurePathController.php @@ -33,18 +33,12 @@ 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. - // On the bright side, we can now differentiate between a missing/failed signature and an expired one. + // 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), @@ -87,28 +81,6 @@ private function getUrl(Request $request, bool $absolute = true): string 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; - // } - /** * Determine if the expires timestamp from the given request is not from the past. * From a24fc9c32c765b5784e5b4fd1193ad284acd0287 Mon Sep 17 00:00:00 2001 From: ildyria Date: Tue, 27 May 2025 21:25:31 +0200 Subject: [PATCH 3/3] fix upstream --- app/Http/Controllers/SecurePathController.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/app/Http/Controllers/SecurePathController.php b/app/Http/Controllers/SecurePathController.php index 0599e04d093..16f8e40785f 100644 --- a/app/Http/Controllers/SecurePathController.php +++ b/app/Http/Controllers/SecurePathController.php @@ -66,19 +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, '?'); + return rtrim($request->url() . '?' . $query_string, '?'); } /**