How are input keys exploitable by malicious users?

Question

In the CodeIgniter PHP framework, there is a function that automatically runs on each request that, among other things, filters the GET/POST/COOKIE array keys, and kills the application if it encounters characters it deems unsafe.

To prevent malicious users from trying to exploit keys we make sure that keys are only named with alpha-numeric text and a few other items.

Something like:

// foreach GET/POST/COOKIE keys as $str...
if ( ! preg_match("/^[a-z0-9:_\/-]+$/i", $str))
{
    exit('Disallowed Key Characters.');
}

For example, this will trigger if you accidentally post something like <input name="TE$T"> or have a query string like ?name|first=1.

I can see this being a good way to enforce common sense key names, or catch mistakes while developing an application, but I don't understand: How can a malicious user possibly "exploit keys" in $_POST data for instance? Especially since (I would assume) input values are just as exploitable, what is this actually preventing?

Answer 1

You see this junk often in noob code:

$_SESSION = $_POST;

A seldom known secret is that $_SESSION is "special" and can't handle the pipe character, |, as a top level array key. php fails to save the session variables during shutdown/session_write_close, instead wiping the entire array.

session_start();

if (!isset($_SESSION['cnt'])) {
    $_SESSION['cnt'] = 0;
}

$_SESSION['cnt']++;

/* prior to php 5.4, it will never increment, because it never successfuly saves
unless you comment line below
*/
$_SESSION['a|b'] = 1;

print_r($_SESSION);

I'm not saying that's why CodeIgniter scrubs the keys, but it's one of many "ways input keys are exploitable".

Maybe a more likely reason though is because people certainly do stuff like this:

if ($formPostDidntValidate) {
    echo "Please fix the form submission errors and try again\n";
    foreach ($_POST as $key => $value) {
        echo "<p>$key<br>
              <input name='$key' value='$value'></p>";
    }
}

Outputting request variables without doing proper context-specific escaping, such as escaping for html contexts, html attribute contexts, or even sql contexts:

$sql = "select * from myTable where 1=1";
foreach ($_POST as $key => $value) {
    $sql .= " and $key = '$value'";
}

I've seen plenty of people escape the value, but not the key when building sql and/or html.

If you don't escape everything, you are easily hacked. Scrubbing values isn't as good as escaping, but it's much better than nothing, and considering how many developers aren't yet sophisticated enough to understand when and how to escape, I can see the attraction to adding automatic request variable scrubbing.

Answer 2

Your question, in itself, brings up a good point: it's unclear what exactly you're being protected against. But there are some popular items it could be addressing:

• magic quotes
• things that could eventually lead to SQL injection
• strings that could be executed by way of shell commands
• things that could conflict with your URL
• things that could conflict with HTML
• things that resemble a directory traversal attack
• cross site scripting (XSS) attacks

But other than those, I really can't think of why you'd always why you'd want to generally protect via preg_match("/^[a-z0-9:_\/-]+$/i", $str).

I've got the feeling that they're overprotecting simply because CodeIgniter is so widely used that they need to protect against things they themselves haven't thought of yet for the sake of their users who may be even less-aware of such attacks than CodeIgniter's developers.

Answer 3

Say in a console I change your form's field name from name="email" to name="email\"); DROP TABLE users;

It's not likely to be a successful XSS attack but I can see something like that doing damage to poorly coded PHP. CI is probably just trying to cover all their bases so they can have a claim to be as XSS-protected as possible.

Answer 4

That kind of check is a waste of time. You only access keys that you expect anyway - and if for some reason you iterate over all elements you will most likely escape them properly for whatever you are going to do with them.

However, looking at the skills of an average newbie PHP programmer (even though members of this species most likely don't use a framework at all) it makes some sense, you can never know what nasty things he's going to do with the code which might even kill his cat without such a check.

The same thing applies to rejecting posts containing e.g. "delete from" as a anti-SQL-injection measure. It can easily cause false positives and if you e.g. use parametrized queries you are safe anyway.

Answer 5

Perhaps it is trying to prevent this attack.

The attack works by using knowledge of how PHP builds its hashing structures to make keys in $_POST which take an arbitrarily long time to process.

I suspect it is probably just trying to prevent the more mundane SQL injection attacks though.