-1

I would like to know if the code below is SECURE and not that it is a good or bad practice, since I want to report if it is vulnerable to directory traversal attack. I do not think if this question is duplicated!

I am working on a penetration testing report for one of my PHP projects and am currently assessing its vulnerability to directory traversal (or path traversal) attacks.

Below is a snippet of the code in question:

$number = $_GET['numberByUser'] ?? null;

if (is_numeric($number) && file_exists($number)) {
    readfile($number);
}

In this code, the $GET['numberByUser'] variable can be manipulated by the user, but it is sanitized with the built-in PHP function is_numeric().

There are only all-numeric filenames in the current directory, which can be accessed by every user. I'd like to know if the is_numeric() protect against "../" or "%2e%2e%2f" or any other tricky payloads which can move me outside from the current directory.

My question is: can a malicious user bypass this filter?

I know how to prevent directory traversal vulnerability in PHP, but I want to know if the code above can be bypassed or not. On the server there are files with number names /var/www/html/numbers/123

From my understanding, is_numeric() should theoretically block directory traversal attacks as it ensures that only numerical values pass through. Yet, this function isn't typically used as a safeguard against such attacks.

I have attempted to bypass this function using several payloads sourced from:

However, these attempts were unsuccessful, leading me to believe that is_numeric() could indeed be a viable deterrent against directory traversal attacks, even if this isn't its intended purpose.

I would appreciate if anyone with further knowledge or experience could confirm whether my understanding is accurate or whether there exist known bypass techniques that I may be overlooking.

Márk Matuz
  • 1
  • 2

1 Answers1

2

is_numeric is one of those functions that has a simple name, and a rather complicated implementation. It attempts to detect lots of different sorts of "numbers", including "5", "-5", "5.0", and even "0.5e-3" (a way of writing "0.0005"). While I can't think of a specific attack in this case, it's probably not the best function to rely on.

If what you want is actually only digits, the function you want is the less memorably named ctype_digit.

On a different note, what you are doing here is not sanitising the input, but validating it - sanitisation generally refers to changing the value in some way; validation is about rejecting the value if it doesn't meet the expected format. Sanitisation is generally harder to get right, but in this case, you could sanitise the input by simply casting it to an integer ($number = (int)$number;) - just remember that the result might be 0 or a negative number.

Finally, the reason you won't see lots of uses of either is_numeric or ctype_digit used in this context is that validation often needs more complex rules, and so things like regular expressions are used. For instance if your filenames were actually "1.txt", "2.txt", etc, you could validate with preg_match('/^\d+\.txt$', $number). (Although, regexes are also hard to get right!)

IMSoP
  • 89,526
  • 13
  • 117
  • 169