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);
?>
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