As the title says i made a script to read pdf files. Only specifical files can be opened. All files last modified till 29-09-2008 can be opened. All files after can't.
Here is my code:
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Stienser Omroeper</title>
</head>
<body>
<?php
$file = 'E:/Omrop/'.$_GET['y'].'/'.$_GET['f'];
$filename = $_GET['f'];
header('Content-type: application/pdf');
header('Content-Disposition: inline; filename="' . $filename . '"');
header('Content-Transfer-Encoding: binary');
header('Content-Length: ' . filesize($file));
header('Accept-Ranges: bytes');
@readfile($file);
?>
</body>
</html>
The $_GET contains y (year for map structure) and f (the filename). If i echo $file after and use the link in run on my pc it works perfectly. In browser i get the message This file is broken and can't be repaired..
Anybody ideas?
This code contains a filesystem traversal vulnerability. You are performing no validation of the arguments that lead to the file. Files on disk are blindly opened and fed to the client.
What if you were on a Unix system? What would happen if someone submitted
?y=&f=../../../etc/passwd
?That doesn't even touch the fact that you aren't doing any sort of sanitization on the user's desired filename for the file. The user could submit entirely bogus data there and get an entirely bogus filename.
This code performs no error checking, and even expressly turns errors off when throwing the file at the user using
readfile
. This is the root of your problem. Nobody has any idea what's going wrong.So, we can fix this.
First things first, you're going to want to do some validation on
y
andf
. You mentioned thaty
is a year, soshould do the trick. By forcing it into an integer, you remove any horibleness there.
f
is going to be a bit more tricky. You haven't given us an idea about what the files are named. You're going to want to add some pattern matching validation to ensure that only valid filenames are looked for. For example, if all the PDFs are named "report_something_0000.pdf", then you'd want to validate against, sayNow that we've got a valid filename and a valid year directory, the next step is making sure the file exists.
If
$file
ended up not being set because the pattern match failed, or if the resulting file path wasn't found, then the script will bail with an error message.I'm going to guess that your problems opening older PDFs are caused by the files not existing or having bad permissions. You're feeding Adobe Reader the right headers and then no data.
You'll also want to perform the same kind of sanity checking on the user-supplied desired filename. Again, I don't know your requirements here, but make sure that nothing bogus can sneak in.
Next, get rid of the @ in front of
readfile
. It's suppressing any actual errors, and you're going to want to see them. Because you probably don't want to see them in the output, make sure to set up an error log instead.Finally... how is this code even working? You're emitting headers in the middle of HTML! Not only that, you're giving explicit content-lengths while doing so. You should be getting a hell of a lot of errors from this. Are you sure that you didn't accidentally copy/paste some code wrong here? Maybe you forgot a section at the top where you're calling
ob_start()
? Regardless, ditch everything before the opening<?php
tag.