Any safety concerns with this code?

[LLLLLLL]

I have a file on my server that I will stream to the browser:

header( "Content-Description: File Transfer" );
header( "Content-Type: application/force-download");
header( "Content-Length: " . filesize( $filename ) );
header( "Content-Disposition: attachment; filename=$filename");
readfile( $filename );

 

$filename is going to be in a location that's not publicly available (there's no URL to it, as it's on the server, and not within public_html or subdirectories).

 

Any safety concerns here? Basically, I'm just curious if a user has any way to steal the file or otherwise access the directory. I don't think so, but I'm just tossing this out here as a general discussion.

 

Thanks!

[khizar]

i think its safe user will not get ur secret url

[the182guy]

The readfile() line needs the secret path on it, unless the file is in the current directory.

 

The part which you need to ensure is secure is the part which defines the $filename. Make sure a user cannot insert anythnig into that, or check the user is allowed access to the file before serving it.

[LLLLLLL]

$filename is a full path to the file.

 

I'm reading the filename out of a mysql database; I don't think there's a security issue there.

 

I guess I was wondering if someone had a way to know where the file is coming from. My directory structure has the public_html directory and the "my_files" directory parallel. So there seems no way for anyone to get to "my_files", as it is only accessible from the server. But when I serve up a file from that directory, I wanted to be sure that there's no way for the user to actually know the path/location of the file.

 

I think I've done this, but I just wanted to ask for other opinions. I'm not 100% sure of possible security issues.

[roopurt18]

Then you should likely change this line:

header( "Content-Disposition: attachment; filename=$filename");

to:

header( "Content-Disposition: attachment; filename=" . basename( $filename) );

[LLLLLLL]

Sorry, why does that change help with security?

[roopurt18]

$filename is a full path to the file.

 

I can't imagine why you'd want to disclose the full path to the client.

[LLLLLLL]

Well I have two questions there:

1 - Can the client see this information?

2 - Even if the client can, does it matter? The file is not directly accessible from the web, as far as I am aware.

 

(Can you tell me how I'm able to see the file information in my browser, by the way?)

[roopurt18]

Yes.  Headers are plain-text key / value pairs sent to the user agent.  There are many, many more types of user agents than just web browsers that will show a sophisticated user all of the headers sent and received.  From a security standpoint I adopt the mindset of "The more this is a black box to the user the better."  That particular header controls the preset file name that appears in the "Save As.." prompt so all it needs in the first place is the file name and not the full path to the file. 

 

Also consider that the full path can reveal more about your box than you intend to.  Let's say you were writing a plug-in for a popular CMS, disclosed the full path, and the attacker saw this:

Content-Disposition: attachment; filename=/var/http/1.3/htdocs/domain.com/cms-1.0.3/protected/thefile.mp3

What can the attacker deduce from this?

1) You run Linux.

2) You're probably using Apache Httpd 1.3

3) You're probably hosting multiple domains

4) You're using version 1.0.3 of the CMS

 

So now what can the attacker do?  They can look for known exploits for your versions of the web server and CMS.  Hypothetically it could also be that this CMS doesn't support PHP 5 until version 1.1.0, therefore the attacker can deduce that you're on a version of PHP less than 5, probably 4.x.x, and then look for known exploits for that as well.

[LLLLLLL]

Fair enough. I guess I will copy the file to a temporary directory and stream from there. This way the user won't know as much about the server. Thanks!

[roopurt18]

No.  You can stream from where it is.  Just change the single line of code I pointed out.

[LLLLLLL]

Pardon my ignorance, but won't that then make the file function fail altogether?

 

http://php.net/manual/en/function.basename.php

Given a string containing a path to a file, this function will return the base name of the file.

 

It seems like it would fail with file not found.

[roopurt18]

header( "Content-Length: " . filesize( $filename ) );  // <-- requires full path
header( "Content-Disposition: attachment; filename=$filename"); // <-- does not require full path
readfile( $filename ); // <-- requires full path

 

Therefore you can change it to this:

header( "Content-Length: " . filesize( $filename ) );
header( "Content-Disposition: attachment; filename=" . basename( $filename ) );
readfile( $filename );

[LLLLLLL]

Oh, well yes that makes sense. Thanks.