Reputation: 151
I'm being told to find the unsafe in this code and find a way to fix it. Someone can help me ?
<?php
$file_url = 'upload/news'.$_GET['file'];
header('Content-type: application/octet-stream');
header("Content-Transfer-Encoding: Binary");
header("Content-disposition: attachment; filename=\"". basename($file_url)."\"");
readfile($file_url);
?>
Upvotes: 1
Views: 307
Reputation: 5801
The $_GET['file']
usage in the $file_url
exposes the code to path traversal attacks. This is quite a dangerous setup, any file in the news
directory can be read, if you place sensitive files like .env
in the news
folder they can be read!
If all content should be public you can use pathinfo
to ensure you only get filenames. Checkout the following example:
$fileName = basename($GET['file']);
$fileUrl = 'upload/news/'. $fileName
header('Content-type: application/octet-stream');
header("Content-Transfer-Encoding: Binary");
header("Content-disposition: attachment; filename=\"". $fileName."\"");
readfile($fileUrl);
basename
is safe to use in this case because it removes any path part that could be used for path traversal
basename("../test.php"); // test.php
basename("."); // .
basename("../.."); // ..
basename("../../test.php"); // test.php
Upvotes: 3