PHP regex vulnerability bet

Question

A coworker today made a bet with me that he knows of a way to supply a specially formatted string that could pass the following regex check and still supply a file name with extension .php or .jsp or .asp:

if (preg_match('/\.(jpeg|jpg|gif|png|bmp|jpe)$/i', $var) && preg_match('/\.(asp|jsp|php)$/i', $var) == false) 
{
    echo "No way you have extension .php or .jsp or .asp after this check.";
}

As hard as I tried myself and searched the net, I was unable to find a flaw that would make such thing possible. Could I be overlooking something? Given that "null byte" vulnerability is dealt with, what else might be the issue here?

Note: In no way am I implying that this code is a full-proof method of checking the file extension, there might be a flaw in preg_match() function or the file contents could be of different format, I just ask the question in terms of regex syntax itself.

EDIT - actual code:

if (isset($_FILES["image"]) && $_FILES["image"]["name"] && preg_match('/\.(jpeg|jpg|gif|png|bmp|jpe)$/i', $_FILES["image"]["name"]) && preg_match('/\.(asp|jsp|php)$/i', $_FILES["image"]["name"]) == false) {
        $time = time();
        $imgname = $time . "_" . $_FILES["image"]["name"];
        $dest = "../uploads/images/";

        if (file_exists($dest) == false) {
            mkdir($dest);
        }

        copy($_FILES['image']['tmp_name'], $dest . $imgname);

    }else{
        echo "Invalid image file";
    }

PHP version: 5.3.29

EDIT: epilogue

Turned out the 'vulnerability' only presents itself on Windows. Nevertheless, it did exactly what my coworker told me it would - passed the regex check and saved the file with executable extension. Following was tested on WampServer 2.2 with PHP 5.3.13:

Passing the following string to the regex check above test.php:.jpg (note the ":" colon symbol at the end of desired extension) will validate it and the function copy() seems to omit everything after the colon symbol including the symbol itself. Again, this is only true for windows. On linux the file will be written exactly with the same name as passed to the function.

Solution 1

There is not a single step or a full direct way to exploit your code but here are some thoughts.

You are passing it to copy() in this example but you have mentioned that you have been using this method to validate file ext awhile now so I assume you had other cases that may have used this procedure with other functions too on different PHP versions.

Consider this as a test procedure (Exploiting include, require):

$name = "test.php#.txt";
if (preg_match('/\.(xml|csv|txt)$/i', $name) && preg_match('/\.(asp|jsp|php)$/i', $name) == false) {
    echo "in!!!!";
    include $name;
} else {
    echo "Invalid data file";
}

This will end up by printing "in!!!!" and executing 'test.php' even if it is uploaded it will include it from the tmp folder - of course that in this case you are already owned by the attacker but let's consider this options too. It's not a common scenario for an uploading procedure but it's a concept that can be exploited by combining several methods:

Let's move on - If you execute:

//$_FILES['image']['name'] === "test.php#.jpg";
$name = $_FILES['image']['name'];
if (preg_match('/\.(jpeg|jpg|gif|png|bmp|jpe)$/i', $name) && preg_match('/\.(asp|jsp|php)$/i', $name) == false) {
    echo "in!!!!";
    copy($_FILES['image']['tmp_name'], "../uploads/".$name);
} else {
    echo "Invalid image file";
}

Again perfectly fine. The file is copied into "uploads" folder - you can't access it directly (since the web server will strip down the right side of the #) but you injected the file and the attacker might find a way or another weak point to call it later.

An example for such execution scenario is common among sharing and hosting sites where the files are served by a PHP script which (in some unsafe cases) may load the file by including it with the wrong type of functions such as require, include, file_get_contents that are all vulnerable and can execute the file.

NULL byte The null byte attacks were a big weakness in php < 5.3 but was reintroduced by a regression in versions 5.4+ in some functions including all the file related functions and many more in extensions. It was patched several times but it's still out there and alot of older versions are still in use. In case you are handling with an older php version you are definitely Exposed:

//$_FILES['image']['name'] === "test.php\0.jpg";
$name = $_FILES['image']['name'];
if (preg_match('/\.(jpeg|jpg|gif|png|bmp|jpe)$/i', $name) && preg_match('/\.(asp|jsp|php)$/i', $name) == false) {
    echo "in!!!!";
    copy($_FILES['image']['tmp_name'], "../uploads/".$name);
} else {
    echo "Invalid image file";
}

Will print "in!!!!" and copy your file named "test.php".

The way php fixed that is by checking the string length before and after passing it to more deeper C procedure that creates the actual char array and by that if the string is truncated by the null byte (which indicates end of string in C) the length will not match. read more

Strangely enough even in patched and modern PHP releases it's still out there:

$input = "foo.php\0.gif";
include ($input); // Will load foo.php :)

My Conclusion: Your method of validating file extensions can be improved significantly - Your code allows a PHP file called test.php#.jpg to pass through while it shouldn't. Successful attacks are mostly executed by combining several vulnerabilities even minor ones - you should consider any unexpected outcome and behavior as one.

Note: there are many more concerns about file names and pictures cause they are many time included in pages later on and if they are not filtered correctly and included safely you expose yourself to many more XSS stuff but that's out of topic.

Solution 2

Try this code.

$allowedExtension = array('jpeg','png','bmp'); // make list of all allowed extension

if(isset($_FILES["image"]["name"])){
     $filenameArray = explode('.',$_FILES["image"]["name"]);
     $extension = end($filenameArray);
     if(in_array($extension,$allowedExtension)){
        echo "allowed extension";
     }else{
          echo "not allowed extension";
     }
}

Comments

[astralmaster]

@SeanBright True. Anything about the issue itself?

[Mariano]

@revo It's D (PCRE_DOLLAR_ENDONLY) php.net/manual/en/reference.pcre.pattern.modifiers.php

[Somnath Muluk]

I got you already. copy is fine. most people do like this. But still was thinking when can anyone include image file in PHP code? (Mainly that could execute the PHP code)

[astralmaster]

Was patched since PHP 5.2.X

[astralmaster]

References for but was reintroduced by a regression in versions 5.4+ in some functions ?

[Shlomi Hassid]

if you restrict it to images than mostly not but its a common beginner mistakes that may serve files back to user of any type by require or include instead of readfile - or in a template base system once you loaded the file you can find a page that loads its template from the based on a url param and than call your injected file... use your imagination :)

[Shlomi Hassid]

@astralmaster php internals and mailing list - but there are some other blogs give me a sec

[Halayem Anis]

No, only since PHP 5.3.X... at least, i've tested my example in PHP 5.2.10 on Windows, and the issue still there :)

[Shlomi Hassid]

Best null byte regression ref: bugs.php hit Ctrl+F and type null bytes - all the path related bug fixes are related to this issue exactly.

[choz]

Why would the attacker use test.php\0.jpg if he can just use test.php.jpg? Sorry, newb here..

[Shlomi Hassid]

@choz read the "read more" I added - its interesting and will get you on track - with test.php\0.jpg regex will accept and copy() will only see the test.php part the rest is truncated - that's relevant only for older versions and for unmaintained copies.

[choz]

@ShlomiHassid Wow.. That's dangerous shit dude.. Thanks for pointing out for me.. +1

[Shlomi Hassid]

@HalayemAnis many older version get patched back especially on well maintained systems that implements security patches - you can build from before a revision and test it yourself.

[Shlomi Hassid]

@choz if you wand dangerous keep track of the bug reports and revisions and maybe you can even contribute. :)