File Download Problem

[Ustag]

Hello, I use this script to download files from my server but if the file contains a "," then the dl.php (download script) will be downloaded instead. Does anybody know why it does that?

<?phpsession_start();include("config.php");$year = date(o);$month = date(F);mysql_query("UPDATE files SET dls=dls + 1 WHERE id=".$_POST['id']."");mysql_query("UPDATE downloads SET $month=$month+1 WHERE Year='$year'");$path = $_SERVER['DOCUMENT_ROOT']."/files/".$_POST['lo']."/"; // change the path to fit your websites document structure$fullPath = $path.$_POST['fn'];if ($fd = fopen ($fullPath, "r")) {    $fsize = filesize($fullPath);    $path_parts = pathinfo($fullPath);    $ext = strtolower($path_parts["extension"]);    switch ($ext) {	    default;	    header("Content-type: application/octet-stream");	    header("Content-Disposition: filename=\"".$path_parts["basename"]."\"");    }    header("Content-length: $fsize");    header("Cache-control: private"); //use this to open files directlywhile(!feof($fd)) {	    $buffer = fread($fd, 2048);	    echo $buffer;    }}fclose ($fd);exit;mysql_close($con);?>

[justsomeguy]

First, that script isn't secure, people can use that to download any file on your server that PHP has access to, even your other PHP files or any server configuration files that PHP can access. You're not validating the $_POST['lo'] or $_POST['fn'] values to make sure they don't contain paths to other files. If the problem that you're describing is that it downloads the code for this script if the filename contains a comma, then I would say to print out the filename you're telling it to open so that you can verify that. You may need to escape special characters in the filename.

[Ustag]

Ok so I made some changes that should make this more secure, now it should only download a file that is in the database and many things have to be accurate for the file to download. But the comma problem still exists and on the page with the download button I print the filename and its no problem. This is how it looks now:

<?phpsession_start();include("config.php");$year = date(o);$month = date(F);$location = mysql_real_escape_string($_POST['lo']);$fname = mysql_real_escape_string($_POST['fn']);$id = mysql_real_escape_string($_POST['id']);$key = mysql_real_escape_string($_POST['ke']);$result = mysql_query("SELECT * FROM `files` WHERE `key`='$key' AND `loc`='$location' AND `filename`='$fname' AND `id`='$id'") or die(mysql_error());$add = mysql_fetch_array($result, MYSQL_BOTH);mysql_query("UPDATE `files` SET `dls`=dls + 1 WHERE `id`=".$add['id']." AND `key`='".$add['key']."' AND `loc`='".$add['loc']."' AND `filename`='".$add['filename']."'");mysql_query("UPDATE `downloads` SET $month=$month+1 WHERE `Year`='$year'");$path = $_SERVER['DOCUMENT_ROOT']."/files/".$add['loc']."/"; // change the path to fit your websites document structure$fullPath = $path.$add['filename'];if ($fd = fopen ($fullPath, "r")) {	$fsize = filesize($fullPath);	$path_parts = pathinfo($fullPath);	$ext = strtolower($path_parts["extension"]);	switch ($ext) {		default;		header("Content-type: application/octet-stream");		header("Content-Disposition: filename=\"".$path_parts["basename"]."\"");	}	header("Content-length: $fsize");	header("Cache-control: private"); //use this to open files directlywhile(!feof($fd)) {		$buffer = fread($fd, 2048);		echo $buffer;	}}fclose ($fd);exit;mysql_close($con);?>

Thank you for your time.

[justsomeguy]

I'm looking for the value of $fullPath in that script, print that out to verify that it doesn't contain any invalid characters and points to the correct file. Don't assume it has the same value as some other variable in some other script. Comment out the headers if you want to test that without having to save and open the result.

[Ustag]

$fullpath prints "/home2/hulk/public_html/filepage/filefolder/username/aha,00.txt" (with some edit so people can't figure out from here what website is not secure and hack it) Nothing strange here I think. And I have to verify this variable so that it only can contain "/home2/hulk/public_html/filepage/filefolder/username/filename.ext"? hmmmm

[justsomeguy]

Commas have special meaning in shell commands, and I'm not sure if fopen automatically escapes the filename to handle that. If not, then it may work to use escapeshellarg on the filename: if ($fd = fopen (escapeshellarg($fullPath), "r")) { Also, it would be good to make sure that all error messages are being printed, at least while you're testing. This will display all errors in the browser: error_reporting(E_ALL);ini_set('html_errors', 1);ini_set('log_errors', 0);ini_set('display_errors', 1); And this will send all errors to a file called error.log in the same directory, as long as PHP has permission to write to it: error_reporting(E_ALL);ini_set('error_log', dirname(__FILE__) . DIRECTORY_SEPARATOR . 'error.log');ini_set('html_errors', 0);ini_set('log_errors', 1);ini_set('display_errors', 0);

[Ustag]

Adding escapeshellarg gives me dl.php as blank page and error message: [09-Nov-2011 08:23:33] PHP Warning: fopen('/home2/hulk/public_html/filepage/filefolder/username/aha,00.txt'): failed to open stream: No such file or directory in /home2/hulk/public_html/filepage/dl.php on line 30[09-Nov-2011 08:23:33] PHP Warning: fclose(): supplied argument is not a valid stream resource in /home2/hulk/public_html/filepage/dl.php on line 48 line 30 is: if ($fd = fopen (escapeshellarg($fullPath), "r")) {line48 is: fclose ($fd); And this is hapening on files even without comma.

[justsomeguy]

OK, that means that fopen is escaping the filename itself if necessary. I don't see the same behavior, if I use your code and remove the database parts and hardcode the filename to download, I get the correct result regardless of the filename. I don't see a reason in your code why it would return the text of the PHP file, it doesn't make sense that it would use a different file other than the one you tell it to and not produce an error message. The only reason the server would return PHP code is if the code wasn't being executed, like if you just double-click a file to open it in the browser instead of accessing it through the server.

[Ustag]

Yes, I hardcoded also and I still get this problem, maybe there is something with my php.ini?

[justsomeguy]

The only reason I know of that fopen would open your PHP file is if you told it to. I don't know of any situation where you tell it to open one file and it would open a different file instead and not return an error message.

[Ustag]

I just notice that jpg files downloaded with this scripts get corrupt while the image works fine when downloading it trough ftp... i go crazy now :'(also now it downloads dl.php with not the script in it but the $fullPath printed in it...

[justsomeguy]

When I run into problems with binary files I use a hex editor to compare the original and changed versions to see which bytes are actually different and why. If PHP is escaping characters or stripping slashes in binary data, that's not a good thing.

[birbal]

if the file is binary use 'b' after mode. it will force to open in binary mode. by default its 't' which will open in text mode and wont treat it as binary.

fopen ($fullPath, "rb")

[Ustag]

I read that too late to try it, sorry, but my solution was to delete this file and find another one that works for me Thank you both for your time!