Page 1 of 1

Are there any security issues with Content-disposition?

Posted: Tue Apr 04, 2006 4:41 am
by Jenk
Hi folks,

I'm in the process of developing a CVS, one of the features of this will be the files contained are served to the user, with the use of header('Content-Disposition: attachment;');

Due to my sucky explanation, hopefully this little snippet will clarify what I am trying to achieve (the actual code is different, but the method is the same):

Code: Select all

<?php


//is_valid_file_id() is a pseudo function to ensure the file is "allowed"
// $files is an array of the files available to download..

if ((isset($_GET['fid'])) && (is_valid_file_id($_GET['fid']))) {
    header('Content-Disposition: attachment; filename="' . basename($files[$_GET['fid']]) . '";');
    readfile($files[$GET['fid']);
} else {
    die('Invalid file specified');
}

?>
now aside from any typo's, missing braces etc. are there any security holes that can be exploited with the above method?

As should be quite clear to see, the CVS will be used to store source files - would there be a way for someone to exploit the disposition (or readfile()) so that instead of the source being served as a download attachment, that it will actually run on the server?

Thanks in advance :)

Posted: Tue Apr 04, 2006 5:29 am
by Maugrim_The_Reaper
Don't see anything off hand - if the files are whitelisted, and you ignore any other request, and the file is not included/eval'd then it seems safe enough. Running a script on a remote server usually requires a include() or eval() somewhere in conjuntion with lax input filtering and path disclosures (usually through errors).

Posted: Tue Apr 04, 2006 3:16 pm
by Ambush Commander
You're whitelisting the input. You should be okay.

Posted: Tue Apr 04, 2006 4:19 pm
by Ollie Saunders
You're whitelisting the input. You should be okay.
is he? it really all depends on the quality of is_valid_file_id(). personally i'd use ctype_alnum(), it might be a little more restricitive but then you know there aren't going to be a file-system hacks in $_GET['fid'].

Posted: Tue Apr 04, 2006 5:31 pm
by Jenk
It's pseudo, i.e. I didn't want to post the entire whitelist function because that wasn't what the question was refering to :)

I could actually have just done:

Code: Select all

if (isset($files[$_GET['fid']]) {
But I prefer to be more specific :)