CODEWITHSUNDEEP
phpsecurityencodingurl-parametersdirectory-traversal
Jon
Jon

Reputation: 437754

Is dirname adequate to prevent directory traversal attacks?

Consider the following classic problem case:

<?php
$filename = "/tmp/".$_GET['f'];
readfile($filename);

This code is vulnerable to a directory traversal attack, for example if the value of $_GET['f'] is ../etc/shadow the contents of that file will be disclosed to the attacker.

There are well-known approaches to prevent this type of attack; I am not asking how to do that. The question is: is the following use of dirname a bulletproof way to prevent the attack?

<?php
if (dirname($_GET['f']) != '.') die ('Attack prevented');

It sounds like it should be since dirname:

So as far as I can tell, the only possible avenue of attack would be to pass data to $_GET['f'] in an encoding such that either the character / or \ (let's not forget Windows) encodes to something that does not contain the ASCII value of the corresponding character and at the same time this encoding has to be transparently supported by the underlying C runtime library's fopen function.

The no-ASCII-value restriction rules out all single-byte encodings, UTF-8, and both flavors of UTF-16; furthermore, since the spec for the C runtime is platform-agnostic the attack could only be applicable to some filesystem that used a "vulnerable" encoding to represent names. Such a filesystem does not, to my knowledge, exist; it would hardly make sense for anyone to create it; and finally PHP would not be hosted on such a hypothetical exotic system even if it existed.

In conclusion, it seems to me that this check is 100% safe. Is there something I missed?

Upvotes: 4

Views: 1410

Answers (1)

Chris Rasco
Chris Rasco

Reputation: 2731

I'm not sure I'd ever make the claim that something is 100% safe. That said, I can't think of an obvious case where this would be unsafe and I tried a ton of permutations against it. That said, you'll want to add a check that $_GET['f'] isn't empty in there. Visiting a page with the above code with no value for f gave me the "Attack prevented" message, which is probably not the desired effect.

<?php
if (!empty($_GET['f']) && dirname($_GET['f']) != '.') die ('Attack prevented');

Upvotes: 1

Related Questions