Does this protect against injection attacks?

Question

Does this protect against SQL injection attacks?

function sanitize($value) {
    // Stripslashes
    if (is_array($value)) {
        if (get_magic_quotes_gpc()) {
            $value = array_map("stripslashes", $value);
        }
        $value = array_map("mysql_real_escape_string", $value);
    } else {
        if (get_magic_quotes_gpc()) {
            $value = stripslashes($value);
        }
        $value = mysql_real_escape_string($value);
    }
    return $value;
}

$_REQUEST = array_map('sanitize', $_REQUEST);
$_GET = array_map('sanitize', $_GET);
$_POST = array_map('sanitize', $_POST);
$_COOKIE = array_map('sanitize', $_COOKIE);

What could I add to sanitize() to protect against cross-site scripting? What other channels would allow attackers to insert malicious code?

With Xss I wouldn't worry about that for storing data. When you are outputting data use htmlspecialchars to protect against XSS. — Belinda, Mar 22 '11 at 17:15
Check out some other topics on that matter. This one is pretty good: http://stackoverflow.com/questions/129677/whats-the-best-method-for-sanitizing-user-input-with-php — Bjoern, Mar 22 '11 at 17:15

score 2 · Accepted Answer · edited May 23 '17 at 11:55

2

The one-word answer would be "yes". However:

If $value is an array that contains other arrays it won't be handled correctly. You should loop over $value make a recursive call to sanitize for each array you find.
It's preferable to use prepared statements instead of doing this. Of course, if you already have a complete application and are not building from scratch this can be problematic.

Finally, the other ways in which someone can subvert your application are cross-site scripting (aka CSS or XSS) and cross-site request forgeries (CSRF). There are lots of resources here on SO and on the internet you can use to get up to speed. As a starting point, protection against XSS involves calling htmlspecialchars on anything you output, while protection against CSRF involves requiring a session-specific id code for each operation your privileged users are allowed to perform on your site.

Array-safe sanitize version

function sanitize($value) {
    if (is_array($value)) {
        foreach($value as &$item) {
            $item = sanitize($item);
        }
    } else {
        if (get_magic_quotes_gpc()) {
            $value = stripslashes($value);
        }
        $value = mysql_real_escape_string($value);
    }
    return $value;
}

Update:

For higher visibility: Bjoern's link to this question ( What's the best method for sanitizing user input with PHP? ) is really good.

edited May 23 '17 at 11:55

Community

1
1

answered Mar 22 '11 at 17:13

Jon

428,835
81
738
806

What should I add to prevent cross site scripting? Something that would strip all html/javascript - is strip_tags() and then htmlspecialchars() a good choice? – A-OK Mar 22 '11 at 17:20
1

@A-OK: `htmlspecialchars` is good and sufficient. I wouldn't trust `strip_tags` for protection because of the possibility of malformed HTML being fed to you on purpose to confuse it. If you want to, you can use `strip_tags` for cosmetic improvement **and then definitely also `htmlspecialchars`** for real security. – Jon Mar 22 '11 at 17:23
Thanks. Is there a point calling htmlspecialchars() in sanitize() to protect against XSS? If malicious code has to be sent through $_REQUEST, $_POST, $_GET or $_COOKIE wouldn't it be effective in stripping any HTML/JS content? – A-OK Mar 22 '11 at 17:29
1

@A-OK: No, do not call `htmlspecialchars` in `sanitize`. Data that goes into HTML should be processed *only with* `htmlspecialchars`, and data that goes into SQL *only with* `mysql_real_escape_string`. It's a very bad idea to call `sanitize` or anything else blindly on e.g. `$_POST`, because what you have to do depends on where the data is going. – Jon Mar 22 '11 at 17:32
I suppose the `&` in `&$item` is a typo? – Nick Jul 08 '13 at 14:35
@Nick: No, it's iterating by reference (see PHP manual `foreach`). – Jon Jul 08 '13 at 15:18

score 2 · Answer 2 · answered Mar 22 '11 at 17:18

2

No.

Use PHP Data Objects Or... Use a Database Abstraction Layer Or... Some framework that does this.

Don't write your own because:

Someone else has
Their code works fine
You can use their code for free
They thought of all the issues you don't know about yet.
It's a lot of work to do this, it's already been done, just spend twenty minutes and figure out someone else's code that does this.

answered Mar 22 '11 at 17:18

Incognito

20,537
15
80
120

Thanks but this would require a bit more work since I'm not making anything from scratch. Also, learning is fun. – A-OK Mar 22 '11 at 17:24
How does using code that's already been written take more time than writing the code yourself? – Incognito Mar 22 '11 at 17:26
Well it depends on how much time it would take to learn about it and use it on an existing website. Does my hosting server support PDO? There's no mention of it in phpinfo(). – A-OK Mar 22 '11 at 17:36
1

Pedantic Note: As to points b and d, just because it exists and is open source, doesn't mean that it actually works and is designed well. I can think of at least one popular database layer out there that's horrifically designed IMHO, yet people use quite often... – ircmaxell Mar 22 '11 at 21:06

score 1 · Answer 3 · answered Mar 22 '11 at 17:17

If it is applied after the database connection was established, then it escapes the initial input data correctly.

Now you will have problems using such escaped values for HTML output however. And it does not protect against second order SQL injection (querying the database, then using those values as-is for a second query). And more importantly, most applications work on the input values. If you do any sort of rewriting or string matching, you might undo some of the escaping.

Hencewhy it is often recommended to apply the escaping right before the query is assembled. Nevertheless, the code itself is functional for the general case and advisable if you can't rewrite heaps of legacy code.

score 0 · Answer 4 · answered Mar 22 '11 at 17:14

0

You should add html_entities. Most of the time you put $_POST variables into a textbox, like:

<textarea><?php echo $_POST['field']; ?></textarea>

They can mess up your HTML by filling in and do anything they want.

answered Mar 22 '11 at 17:14

www.data-blogger.com

4,076
7
43
65

Now they can fill in things like: And mess up your HTML. – www.data-blogger.com Mar 22 '11 at 17:15

Does this protect against injection attacks?

4 Answers4