Processing checkboxes with PHP

Question

My boss set up a script to process forms through PHP and email the results. We are required to use this processor. The problem with the processor is this line of code:

foreach ($_POST as $k => $v){$$k = strip_tags($v);}

This would be fine if all values sent were just strings, but I am trying to process some checkboxes which are passed as arrays. From what I understand, the strip_tags function only works with strings. It processes everything and sends the results via email as it should, but it throws a notice every time it tries to process a series of checkboxes. Notice: array to string conversion... The process still works, I just get ugly notices all over the place. In order to temporarily fix the problem, I removed the strip_tags function, resulting in this:

foreach ($_POST as $k => $v){$$k = $v;}

Everything now functions properly and I get no warnings, errors or notices. However, after pointing this out to my boss he wants me to revert back to the original code and then give each checkbox its own unique name, instead of giving them all the same name with different values. I could do that, but I know that is not the proper way to process a series of checkboxes. Plus it creates all sorts of headaches. My boss simply does not understand how to work with arrays, so he comes up with stupid work-arounds like this every time he encounters one. He also claims that this is some sort of spam protection to stop people from adding recipients to our forms. I may not be an expert in PHP, but I'm pretty sure that statement is false.

So what can I do to fix this issue? I know I should be converting the checkbox arrays to strings first, then use the strip_tags function on the resulting strings, but I am still fairly new to PHP and don't entirely understand what that line of code is doing to begin with. Can anybody help at least point me in the right direction?

Answer 1

Point out to your boss that:

<input type="checkbox" name="valid_recipient[]" value="1" /> [email protected]
<input type="checkbox" name="valid_recipient[]" value="x" /> [email protected]

and

<input type="checkbox" name="valid_recipient1" value="1" /> [email protected]
<input type="checkbox" name="valid_recipient2" value="x" /> [email protected]

are the same thing, whether they get passed as an array of checkbox values, or individual checkbox/value pairs. Either way, Mr. Nasty just injected something into your checkbox list.

As well, what's a malicious user from setting

<input type="hidden" name="_POST" value="haha I wiped your post array!" />

into the form. Your PHB's handy dandy "makes us completely secure" processor has just happily nuked the $_POST array, all while being "completely secure"

Answer 2

Passing checkboxes as an array is kush:

<input type="checkbox" name="myarray[key1]" value="hello world" />
<input type="checkbox" name="myarray[key2]" value="well hello again" />

Will create a resultant $_POST like:

Array
(
    [myarray] => Array
        (
            [key1] => hello world
            [key2] => well hello again
        )

)

While this is awesome it also means that your bosses code is doubly insecure:

First, not having the strip_tags in there makes you vulnerable to XSS attacks.

Second, naively trusting $_POST variable names and exporting them into the global namespace is a recipe for disaster. (There's a reason why register_globals is a thing of the past).

For instance, let's say you track the username of who is logged in using a simple $_SESSION variable. Something like this:

if ($_SESSION['logged_in_user'] == 'admin') {
   // do administrator things
}

Well, by accepting and exporting $_POST variables and attacker could modify an HTML form element like this:

<input type="checkbox" name="myarray[key1]" value="hello world" />

And twiddle it into something like this (using Firebug or Chrome):

<input type="checkbox" name="_SESSION[logged_in_user]" value="admin" />

Tad-ah! Any anonymous user can gain administrator access to the web site.

Here is a simple script for your consideration:

<pre>
<?php 
session_start();
$_SESSION['logged_in_user'] = 'pygorex1';
print_r($_SESSION);
foreach ($_POST as $k => $v) {
    $$k = $v;
}
?>
</pre>
<form method="post">
<input type="checkbox" name="myarray[key1]" value="hello world" />
<input type="checkbox" name="_SESSION[logged_in_user]" value="admin" />
<input type="submit" value="go go gadget" />
</form>
<pre>
<?php 
print_r($_SESSION);
print_r($myarray);
session_write_close();

if ($_SESSION['logged_in_user'] == 'admin') {
    echo("OWNED\n");
}

?>
</pre>