# Am I missing any security loopholes in this php script that reads and outputs directory/file from $_GET variable Due to a weird server setup on one of my client's websites, I needed to setup a script to load referenced pdf files through a case-insensitive lookup. We originally looked into mod_speling, but it was causing issues with other mod_rewrite declarations. I'm not entirely sure why, but I didn't have much control over those other declarations, since they had other people working on their site as well. So anyway, they have a bunch of references to their PDF files all over the internet with a bunch of different capitalization. Realizing that getting all of these to switch over to the proper file name was gonna be nearly impossible, I opted to write a script to do the translation for me. The script forces http://www.somedomain.com/somedirectory/subdirectory/anypdffile.pdf to load through my pdffiles.php file. The script seems to work perfectly. I'm not too worried about server performance, as I don't think it's gonna be used all that often. I just wanted to get a second pair of eyes to make sure I didn't leave any major security loopholes in here since I'm reading a directory/file from a$_GET variable.

First I added this to the .htaccess file to redirect all pdf files to a php script

RewriteEngine on
RewriteBase /
RewriteCond %{REQUEST_URI} \.pdf$[NC] RewriteRule (.*) pdffiles.php?p=$1


Then put the following code in the pdffiles.php file:

<?php

// shorthand for our docroot
$root =$_SERVER['DOCUMENT_ROOT'];

// get our filepath (correct format is "somedir/otherdir/xyz.pdf")
$path=$_GET['p'];

// Rule 1 - Don't allow empty path
if (!empty($path)) { // get the directory for our file$dir = dirname($root.'/'.$path);

// get the filename
$file = basename($path);

// get our extension
$ext = pathinfo($file, PATHINFO_EXTENSION);

if (!strstr($_SERVER['REQUEST_URI'], 'pdffiles.php')) { // Rule 3 - Ensure PDF extension if (strtolower($ext) == "pdf") {

// Rule 3 - Make sure document root + path is a real path
if (($dir = realpath($dir))) {

// Loop through directory contents
if ($fh = opendir($dir)) {
while (false !== ($tfile = readdir($fh))) {

// ignore the default directories
if ($tfile != "." &&$tfile != "..") {

// convert both to all lower case and try to match
if (strtolower($tfile) == strtolower($file)) {

// if they match then go ahead and output file
readfile($dir.'/'.$tfile);
exit;

}
}

}
}

}
}

}
}

// we failed one of our checkpoints, redirect to 404
?>

-

## migrated from stackoverflow.comJan 22 '13 at 4:50

This question came from our site for professional and enthusiast programmers.

Ack, sorry, didn't realize there was a code review section. Can I switch it? –  David Stinemetze Jan 21 '13 at 20:50
looks good to me –  cubsink Jan 21 '13 at 20:50
A mod can do it for you. Just flag it and they'll do the rest. –  John Conde Jan 21 '13 at 20:51
K. Thanks @JohnConde. My bad. –  David Stinemetze Jan 21 '13 at 20:51
Not sure if it's an exploitable issue, but what would happen if someone set p to a relative path like ../cgi/example.pdf? I can't forsee any issues though as the pdf extension check will stop you mistakenly serving up internal files. –  Jason Larke Jan 22 '13 at 5:50

As Jason stated checking for ".." is a good idea. If there are only a limited number of path where the pdfs are located on the server, I would even prefer matching against this set.

Nevertheless, you might also want to invert your ifs to get rid of the nesting and use guard conditions.

<?php
// !!!! comment intention, not what your code is doing

if (strstr($_SERVER['REQUEST_URI'], 'pdffiles.php')) notFound(); // expected format is "somedir/otherdir/xyz.pdf"$path= $_GET['p']; if (empty($path)) notFound();

$root =$_SERVER['DOCUMENT_ROOT'];
$dir = realpath(dirname($root.'/'.$path)); if ($dir===FALSE) notFound();
$file = basename($path);
$ext = pathinfo($file, PATHINFO_EXTENSION);
if (strtolower($ext) != "pdf") notFound(); if ($fh = opendir($dir)) { while (false !== ($tfile = readdir($fh))) { if ($tfile == "." || $tfile == "..") continue; if (strtolower($tfile) != strtolower($file)) continue; header("Content-type: application/pdf"); readfile($dir.'/'.\$tfile);
exit;
}
}

function notFound() {
// we failed one of our checkpoints, redirect to 404