4

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:

  • returns . if and only if there are no slashes in the input path (the online documentation makes a less rigorous guarantee but the source is explicit)
  • is binary-safe (so cannot be tricked by embedded nulls)

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?

Jon
  • 428,835
  • 81
  • 738
  • 806
  • I have used the code from this [answer](http://stackoverflow.com/a/4205278/453348) also you can check the `MainMa` answer from that question – tttony Oct 18 '13 at 19:29
  • @tttony: Thanks, but as I say above I know how to prevent the attack. The question is, can it be confidently prevented *this way*? – Jon Oct 18 '13 at 20:01
  • According to my test, your method works, look simple but it works – tttony Oct 18 '13 at 23:54

1 Answers1

1

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');
Chris Rasco
  • 2,713
  • 1
  • 18
  • 22
  • Sure, the examples contain the bare minimum code to illustrate. What kind of permutations did you try? – Jon Oct 18 '13 at 18:59
  • I created a file 1 level up and typed in every conceivable way I could think of to reference that file (i.e. ?f=../test.txt ?f=..\test.txt ?f=..%2Ftest.txt ?f=/test.txt ?f=%2Ftest.txt ?f=asdf) The list goes on and on. Only file paths that were truly local to the script didn't throw the error. Again, I hate to use "100% safe", but I wasn't able to find something obvious to get past that. – Chris Rasco Oct 18 '13 at 22:08