Skip to content
New issue

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

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

Already on GitHub? # to your account

security: use signed middleware #197

Merged
merged 2 commits into from
Aug 12, 2024
Merged

security: use signed middleware #197

merged 2 commits into from
Aug 12, 2024

Conversation

pxlrbt
Copy link
Owner

@pxlrbt pxlrbt commented Aug 12, 2024

fix: use signed URL

The current version might allow a path traversal attack with badly configured webservers through the route for the export file download, because the route was using {path} and ->where('path', '.*').

Thanks to Kevin Pohl for making me aware of this issue.

@pxlrbt pxlrbt merged commit 0d6f3f2 into main Aug 12, 2024
Route::get('filament-excel/{path}', function (string $path) {
$path = Storage::disk('filament-excel')->path($path);
$filename = substr($path, 37);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pxlrbt you broke file downloading functionality with this change. Initial $path variable contains UUID + filename. But Storage::disk('filament-excel')->path($path) contains absolute path for $path. It leads to The filename and the fallback cannot contain the "/" and "\" characters. exception.

For example, if initial $path contains d98006dc-0cf4-41b0-8381-1cb6c9521ba0-filename.xls, then Storage::disk('filament-excel')->path($path) contains /var/www/html/storage/app/filament-excel/d98006dc-0cf4-41b0-8381-1cb6c9521ba0-filename.xls. If you substr the first one, you'll get actual filename. If you substr the second one, you'll receive cel/d98006dc-0cf4-41b0-8381-1cb6c9521ba0-filename.xls which leads to an exceptions.
Fix this, please.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@InfluxOW Thanks for reporting. I fixed the order in #198.

pxlrbt added a commit that referenced this pull request Aug 19, 2024
pxlrbt added a commit that referenced this pull request Aug 19, 2024
pxlrbt added a commit that referenced this pull request Aug 19, 2024
deerf0x pushed a commit to deerf0x/filament-excel-v1 that referenced this pull request Sep 4, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants