Local File Inclusion by one misplaced character

Tags: security, websites.
By lucb1e on 2015-10-29 13:08:44 +0100

"Uh-oh," I thought, as a good friend alerted me to a local file inclusion vulnerability in a site that I recently made. It's still in beta, not public yet, but still. How could this happen? I wrote code to prevent this!

The site has a ?page=mypage parameter that you could modify into anything, and it would just include the page instead of restricting it to existing and allowed pages. An absolute beginner's mistake, and I had thought of it, and secured it. Just not tested it.

I went to look at the code.

if (in_array($_GET['page'], array('about', 'contact', true))) {

    require($_GET['page'] . '.php');

}

Can you spot the error?

I'll leave a bit blank here to give you a chance to spot the error.




The way I tried to secure is by saying that the value for the ?page= parameter has to be in an array of whitelisted values. Now I know that PHP's in_array() function does loose comparison by default, and I don't want that. You can make it use strict comparison by doing in_array($val, $arr, true) — so passing a final "true" parameter to in_array.

By misplacing the ) sign (closing parenthesis), I accidentally made it even worse than loose comparison: every string is loosely equal to true!

"random/path" == true // works

"random/path" === true // does not work (=== is strict comparison)

So if ?page=random/path is loosely equal to true, and "true" is in that array of allowed values, then all values are allowed! Local file inclusion!

This wasn't the end of the world: I had open_basedir in effect and very strict; the site was not too interesting; the admin page was properly secured; and you couldn't view things like database credentials because it's an include, not a dump. Still, a terribly stupid mistake by one misplaced character.

For completeness, the correct code is this (spot the difference!):

if (in_array($_GET['page'], array('about', 'contact'), true)) {

    require($_GET['page'] . '.php');

}