tedd
tedd

Reputation: 151

how to set headers in php for safety

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

Answers (1)

MaartenDev
MaartenDev

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

Related Questions