Skip to content

Commit d9c363a

Browse files
authored
Revert part of #3373 (#3377)
1 parent 06e00cf commit d9c363a

1 file changed

Lines changed: 9 additions & 40 deletions

File tree

app/Http/Controllers/SecurePathController.php

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -33,25 +33,19 @@ class SecurePathController extends Controller
3333

3434
public function __invoke(Request $request, ?string $path)
3535
{
36-
// In theory we should use the `$request->hasCorrectSignature()` method here.
37-
// However, for some stupid unknown reason, the path value is added to the server Query String.
38-
// This completely invalidates the signature check.
39-
// For example the url http://localhost:8000/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png?expires=1748380289
40-
// will be verified as :
41-
// http://localhost:8000/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png?/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png&expires=1748380289
42-
// which makes the signature check fail as the hmac does not match.
43-
if (!self::shouldNotUseSignedUrl() && !$this->hasCorrectSignature($request)) {
36+
// First we verify that the request has not expired.
37+
if (!self::shouldNotUseSignedUrl() && !$this->signatureHasNotExpired($request)) {
38+
throw new SignatureExpiredException();
39+
}
40+
41+
// Then we verify that the request has a valid signature.
42+
if (!self::shouldNotUseSignedUrl() && !$request->hasValidSignature()) {
4443
Log::error('Invalid signature for secure path request. Verify that the url generated for the image match.', [
4544
'candidate url' => $this->getUrl($request),
4645
]);
4746
throw new InvalidSignatureException();
4847
}
4948

50-
// On the bright side, we can now differentiate between a missing/failed signature and an expired one.
51-
if (!self::shouldNotUseSignedUrl() && !$this->signatureHasNotExpired($request)) {
52-
throw new SignatureExpiredException();
53-
}
54-
5549
if (is_null($path)) {
5650
throw new WrongPathException();
5751
}
@@ -72,41 +66,16 @@ public function __invoke(Request $request, ?string $path)
7266
return response()->file($file);
7367
}
7468

75-
private function getUrl(Request $request, bool $absolute = true): string
69+
private function getUrl(Request $request): string
7670
{
7771
$ignore_query = ['signature'];
7872

79-
$url = $absolute ? $request->url() : ('/' . $request->path());
80-
8173
$query_string = '';
8274
$query_string = (new Collection(explode('&', (string) $request->server->get('QUERY_STRING'))))
8375
->reject(fn ($parameter) => in_array(\Str::before($parameter, '='), $ignore_query, true))
84-
->reject(fn ($parameter) => count(explode('=', $parameter)) === 1) // Ignore parameters without value => avoid problem above mentionned.
8576
->join('&');
8677

87-
return rtrim($url . '?' . $query_string, '?');
88-
}
89-
90-
/**
91-
* Determine if the signature from the given request matches the URL.
92-
*
93-
* @param \Illuminate\Http\Request $request
94-
* @param bool $absolute
95-
*
96-
* @return bool
97-
*/
98-
private function hasCorrectSignature(Request $request, bool $absolute = true): bool
99-
{
100-
$original = $this->getUrl($request, $absolute);
101-
$key = new \SensitiveParameterValue(config('app.key'));
102-
if (hash_equals(
103-
hash_hmac('sha256', $original, $key->getValue()),
104-
$request->query('signature', '')
105-
)) {
106-
return true;
107-
}
108-
109-
return false;
78+
return rtrim($request->url() . '?' . $query_string, '?');
11079
}
11180

11281
/**

0 commit comments

Comments
 (0)