Page 1 of 1
Header field from _GET
Posted: Thu Aug 16, 2007 1:22 pm
by GloriousEremite
I wanted to allow visitors of a page to click on an image to download that image, but a simple link only redirected their browser rather than displaying the download dialog, so I came up with this script:
Code: Select all
<?php
$file = $_GET['file'];
$userfilename = $_GET['saveas'];
$type = $_GET['type'];
header('Content-Type: '.$type);
header('Content-Disposition: attachment; filename="'.$userfilename.'"');
readfile($file);
?>
Not being very experienced with php, I thought it best to ask if there are any security concerns here?
Posted: Thu Aug 16, 2007 2:27 pm
by stereofrog
Yes, this code enables HTTP response splitting
http://en.wikipedia.org/wiki/HTTP_response_splitting
and, even worse, allows the attacker to read arbitrary files from your server.
Posted: Fri Aug 17, 2007 6:12 am
by Mordred
Also it can be used to turn your script into a web proxy.
http://php.net/manual/en/features.remote-files.php
Posted: Fri Aug 17, 2007 12:31 pm
by GloriousEremite
>Yes, this code enables HTTP response splitting
That Wikipedia page says, "It is worth noting that although this is not a PHP specific problem, the PHP interpreter contains protection against this attack since version 4.4.2 and 5.1.2 [1]."
That makes it sound like I'm automagically protected...or do I still have to sanitize the string?
>and, even worse, allows the attacker to read arbitrary files from your server.
>Also it can be used to turn your script into a web proxy.
What if I check that the file starts with "./tmp" (the folder the files should be stored in) and contains only a single '.' character?
Posted: Fri Aug 17, 2007 12:38 pm
by miro_igov
Only if allow_url_fopen is On
If you check for ./tmp then is ok but make sure it will not pass ./tmp/../config.php

Posted: Fri Aug 17, 2007 12:39 pm
by feyd
GloriousEremite wrote:That makes it sound like I'm automagically protected...or do I still have to sanitize the string?
I wouldn't expect PHP to magically protect you.
Posted: Sat Aug 18, 2007 12:08 am
by GloriousEremite
"From now on, all new PHP versions will no longer support multiple headers in the header() call and therefore all vulnerable applications will only be exploitable on hosts with old PHP versions." (
php-security.org)
"This function [header()] now prevents more than one header to be sent at once as a protection against header injection attacks." (
php.net)
Just so I get this straight, is there anything to worry about if an attacker can't send multiple headers by inserting a CRLF? (That is, if I'm running the latest version of PHP or remove them manually)
Here's my updated version:
Code: Select all
<?php
$file = $_GET['file'];
$userfilename = $_GET['saveas'];
$type = $_GET['type'];
//prevent HTTP response splitting (header() shouldn't allow it in 5.1.2/4.4.2, but just in case)
if(count($c=preg_grep('/(\r)|(\n)/',$_GET))!==0)
exit("Invalid String!");
//only allow files in ./tmp directory (and only extensionless files)
if (strpos($file,"./tmp")!==0||strpos($file,'.',1))
exit("Invalid File!");
header('Content-Type: '.$type);
header('Content-Disposition: attachment; filename="'.$userfilename.'"');
readfile($file);
?>
Posted: Sat Aug 18, 2007 5:03 am
by timvw
- The code for verifying where the file exists is flawed... Use the realpath function to determine where the file *really* lives...
- The filename in the header should not contain the path..
Posted: Sat Aug 18, 2007 12:19 pm
by GloriousEremite
How exactly is it flawed? I could do something like this:
Code: Select all
if (dirname(realpath($file))!=realpath("./tmp"))
exit("Invalid File!");
Is that what you meant? It just seems like realpath will have to do uneccessary processing.
And the filename in the header doesn't contain the path. It's just the name that is displayed in the download dialog.
Posted: Sat Aug 18, 2007 3:56 pm
by timvw
Simply request page = /etc/motd\x00 and enjoy

Posted: Sat Aug 18, 2007 7:01 pm
by GloriousEremite
I think I must be incredibly dense, but I don't see the problem. Right now I'm testing on a windows box, so that's not a possibility, but anyways it shouldn't pass validation using my code since /etc/motd\x00 doesn't start with './tmp'...
Posted: Sun Aug 19, 2007 1:04 am
by timvw
I thought i had editted my post to: "Search the web for 'php null byte' and see how it can be exploited".
(On second thought: it seems this bug has been solved for a while now)