views:

183

answers:

3

I have a PHP app that uses a $_GET parameter to select JS/CSS files on the filesystem.

If I deny all requests in which the input string contains ./, \ or a byte outside the visible 7-bit ASCII range, is this sufficient to prevent parent directory traversals when the path is passed to PHP's underlying (C-based) file functions?

I'm aware of null-byte vulnerabilities, but are there any other alternative/malformed character encodings tricks that might squeak by these checks?

Here's the basic idea (not production code):

$f = $_GET['f']; // e.g. "path/to/file.js"

// goal: select only unhidden CSS/JS files within DOC_ROOT
if (! preg_match('@^[\x20-\x7E]+$@', $f)     // outside visible ASCII
   || false !== strpos($f, "./")             // has ./
   || false !== strpos($f, "\\")             // has \
   || 0 === strpos(basename($f), ".")        // .isHiddenFile
   || ! preg_match('@\\.(css|js)$i@', $f)    // not JS/CSS
   || ! is_file($_SERVER['DOCUMENT_ROOT'] . '/' . $f)) {
    die();
}
$content = file_get_contents($_SERVER['DOCUMENT_ROOT'] . '/' . $f);

Update: My question is really about how the C filesystem functions interpret arbitrary ASCII sequences (e.g. if there are undocumented escape sequences), but I realize this is likely system-dependent and perhaps unanswerable in practice.

My active validation additionally requires that realpath($fullPath) start with realpath($_SERVER['DOCUMENT_ROOT']), ensuring that the file is within the DOC_ROOT, but a goal of this posting was to ditch realpath() (it's proven unreliable in various environments) while still allowing unusual, but valid URIs like /~user/[my files]/file.plugin.js.

+4  A: 

When filtering input for security, always use whitelists, not backlists.

You should reject all paths that don't match /^([A-Za-z0-9_-]+\/?)*[A-Za-z0-9_-]+\.(js)|(css)?$/.

This will only allow normal segmented paths where each segment has letters, numbers, or _-.

SLaks
Why was this downvoted?
SLaks
+1  A: 

Might require a little rearchitecting, but even if you are passed ../../passwd, basename() will insulate it. Then, you could place all of the files you want to serve in one folder.

Given ../../././././a/b/c/d.txt, basename($f) will be d.txt; this approach seems wiser to me, instead of trying to outsmart the user and forgetting a hole.

Jed Smith
He wants to server subdirectories.
SLaks
@SLaks: hence: "Might require a little rearchitecting"
Jed Smith
A: 

You mention it yourself, but comparing realpath of the input to a known root is the best solution I can think of. Realpath will resolve any hidden features of the path/filesystem, including symlinks.

troelskn
Yeah, I think I'm going to have to live with `realpath`'s peculiarities.
mrclay