Skip to content

Revert part of #3373 #3377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ildyria merged 3 commits into from
May 27, 2025
Merged

Revert part of #3373 #3377

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dbda2bc
maybe?
ildyria May 27, 2025
54e88e1
remove unnecessary comment
ildyria May 27, 2025
a24fc9c
fix upstream
ildyria May 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 9 additions & 40 deletions app/Http/Controllers/SecurePathController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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, '?');
}

/**
Expand Down