PHP auto generate parameters for PDO statement by looping through $_POST global

Question

I have written this function below to return the values from the $_POST global that can be used in the PDO statement. I just want suggestions if this is a good way to do it. I know that the "implode" part might not be very flexible but i would like to know how this can be improved. Any help with the logic and improving the function is deeply appreciated. Thanks.

/**
 * This function loops through the $_POST global and returns parameters that can be used in
 * a PDO statement directly. Note : For this function to work properly the 
 * PDO::ATTR_EMULATE_PREPARES should be set to "false"
 * like so "$conn->setAttribute(PDO::ATTR_EMULATE_PREPARES, false)".
 * @param  Array $exclude This is an array of keys in $_POST that you want the function to ignore
 * @return Array The function returns an array that can be used as parameters for the PDO statement
 */
function get_params($exclude = array()) {
    $keys = array();
    $values = array();
    $placeholder_keys = array();
    $params = array();

    foreach ($_POST as $key => $value) {
        if(!in_array($key, $exclude)) {
            $keys[]             = $key;
            $placeholder_keys[] = ":" . $key;
            if(is_array($value)){
                $value = implode(",", $value);
            }
            $values[] = $value;
        }
    }

    $comma_sep_keys = implode(",", $keys);
    $comma_sep_placeholder_keys = implode(",", $placeholder_keys);

    $params['keys'] = $keys;
    $params['values'] = $values;
    $params['placeholder_keys'] = $placeholder_keys;
    $params['comma_sep_keys'] = $comma_sep_keys;
    $params['comma_sep_placeholder_keys'] = $comma_sep_placeholder_keys;

    return $params;
}

[Insert/update helper function using PDO](http://stackoverflow.com/a/3921798/285587) — Your Common Sense, Jun 09 '13 at 13:16
You could easily make this a lot more safe when you use whitelisting instead of blacklisting. — Arjan, Jun 09 '13 at 14:57

score 4 · Accepted Answer · answered Jun 09 '13 at 13:15

4

You're not sanitizing the keys at all. What if an array element is:

array(
    "foo = ''; DROP TABLE users; --" => 'baz'
)

This leaves you wide open to SQL injection. You're placeholding the values, but in return you're blindly concatenating unsanitized keys into your queries.

You're also imploding array values into a single string; do you really want to insert them as the single value "foo,bar,baz" when they were an array originally?

answered Jun 09 '13 at 13:15

deceze

510,633
85
743
889

Thanks for the feedback. Yes i want to insert the comma separated list.. Do i really need to sanitize the values even when i am using prepared statements? thanks – Lucky Soni Jun 09 '13 at 13:20
You need to sanitize the **keys**, because you can't prepare *them*. – deceze Jun 09 '13 at 14:53
Usually inserting a comma separated list is an indication of bad database design, though we would need more information about the use case before we can make any better statements about that. Also, as @deceze indicated, you need to sanitize the keys. You're doing ok with the values, but not with the keys. – Arjan Jun 09 '13 at 14:56

score 1 · Answer 2 · edited May 23 '17 at 11:52

1

Expanding a bit to deceze in case it wasn't clear (and because that's the first thing I thought), your user could change the form to:

<input name = "foo = ''; DROP TABLE users; --" value = 'baz'>

Also, you are very vulnerable to XSS attacks. What if someone entered as a value this:

<SCRIPT SRC=http://ha.ckers.org/xss.js></SCRIPT>

Then, everyone that enters in the page where that user's input can be read (most importantly, you), will load that xss.js. This is very dangerous, and I recommend having a strict sanitizing rules. EDIT: Normally you should use htmlentities(), but only use them when outputting data, as can be learned in this question in SO. But, if you are accepting html, a good library for it is HTML Purifier.

edited May 23 '17 at 11:52

Community

1
1

answered Jun 09 '13 at 13:35

Francisco Presencia

8,732
6
46
90

Wrong wrong wrong! You should enter the script tag as is and rather correctly - based on the context. The **only** situation where you need something like HTML Purifier is when your input is html (wysiwyg editors only in general). And even in that case that should happen the moment you output it in a html context, *not* when you enter it in the database. – David Mulder Jun 09 '13 at 13:41
I knew about HTML Purifier only for html, I just don't know what I was thinking when I answered... and I just learned about purifying the output when you display it, thanks for making me a better programmer @DavidMulder , I edited your suggestions. Anything else to be improved? – Francisco Presencia Jun 09 '13 at 14:47

PHP auto generate parameters for PDO statement by looping through $_POST global

2 Answers2