Are there any security issues with Content-disposition?

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Are there any security issues with Content-disposition?

Post 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 :)
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post 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).
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

You're whitelisting the input. You should be okay.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post 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'].
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post 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 :)
Post Reply