Header() Save As Taking Too Long

[Jonah-B]

I have a "save file" button for mp3 files on my site. I've never used the header() function before, so I'm not positive that I'm using it right here. It works perfectly on localhost. When I upload it to my web host it works, but it takes forever for the save dialog to appear. You click the save file button and nothing happens until 20+ seconds later, then the dialog appears. It seems that the browser is downloading the whole file before the save dialog appears, then after selecting a destination the download happens instantaneously. As I've said, I'm not too familiar with the header() function or how it works, so what can I do to pop up the save as dialog first?

 

Here is the download.php

 

<?php
header('Pragma: public');
header('Expires: 0');
header('Cache-Control: must-revalidate, post-check=0, pre-check=0');
header('Cache-Control: public');
header('Content-Description: File Transfer');
header('Content-Type: application/force-download');
header('Content-Disposition: attachment; filename="'.basename($_GET['f']).'"');
header('Content-Transfer-Encoding: binary');
readfile($_GET["f"]);
?>

 

Thanks!

[requinix]

If that's what's happening (and given your description it sounds possible) then that's the browser's fault. Besides the HUGE SECURITY HOLE the only thing I see missing is setting the content-length.

header('Content-Length: ' . filesize($_GET["f"]));

 

As for the more important problem, your script will allow anyone to download any file on your server. All I have to do is change the URL like

script.php?f=/etc/passwd

And now I have a list of all the users on your machine. And if one of those users is "jonahb" then

script.php?f=/home/jonahb/.htpasswd

to grab your password list (for example).

 

You need to validate the filename. You cannot just allow anything. If all the files are uploads/*.mp3 then validate that:

if (dirname($_GET["f"]) != "uploads" || !fnmatch("*.mp3", basename($_GET["f"]))) {
   // invalid file! they shouldn't be downloading this!
}

And you should check that the file actually exists, of course. And that $_GET["f"] actually exists (otherwise you'll get "undefined offset" warnings from PHP).

Edited October 15, 2012 by requinix

[Jonah-B]

Thank you requinix! I didn't even think of such a security hole. I'll definitely fix that. I'm still a noob I guess, but we've all gotta start somewhere

 

I'll try plugging the content-length into the header and see if that makes a difference. The browser never behaves like this for other file downloads so thats why I suspected the code. The browser that I tested this with was Google Chrome.

 

Again, thank you for the helpful reply!