PHP Downloading a file using url query - what are all the threats?

Question

Had no luck finding an answer on the internet so I figured I'd ask the experts. What are the security threats that come with downloading files using url query, for example a code like this:

if(isset($_SERVER['QUERY_STRING']) && $_SERVER['QUERY_STRING'] != ""){
    $file = urldecode($_SERVER['QUERY_STRING']);
    if(file_exists("files/".$file)){
        header('Content-Description: File Transfer');
        header('Content-Type: application/octet-stream');
        header("Content-Transfer-Encoding: Binary");
        header('Content-Disposition: attachment; filename="'.basename($file).'"');
        header('Expires: 0');
        header('Cache-Control: must-revalidate');
        header('Pragma: public');
        header('Content-Length: ' . filesize("files/".$file));
        ob_clean();
        flush();
        readfile("files/".$file);
        exit;
    }
}

would be vulnerable to query strings like "?..\..\..\any\file\here" but isn't that all? Can't this be filtered out by a single if?

    $file = urldecode($_SERVER['QUERY_STRING']);
    if(strpos($file, ".\\") !== false || strpos($file, "./") !== false){
        echo "No you won't get my system files";
        exit;
    }

If there's no "upload" feature, a .htaccess file preventing access to \files\ directory and the "if strpos" then is this safe?

Edit: bad code example

Please note that a clever hacker can circumvent your security filter by using forward slashes. That's the problem of black list approaches: you can't figure out all possible cases. — Álvaro González, Mar 24 '18 at 11:20

score 0 · Accepted Answer · answered Mar 24 '18 at 11:23

1) You would be vulnerable to ..\..\..\any\file\here

2) I would recommend you to get a list of all sub-files in files/. Then if the file requested does not match any of those files return a 404 Error code.

PHP read sub-directories and loop through files how to?

PHP Downloading a file using url query - what are all the threats?

1 Answers1