4
votes

I'm letting users update their name with this code.

    $dbh = connect();
    $q = $dbh->prepare('UPDATE Users SET username=:name WHERE User_ID=:id LIMIT 1'); 
    $q->bindParam(":id", $loggedInUser->user_id, PDO::PARAM_INT);
    $q->bindParam(":name", $_GET['name'], PDO::PARAM_STR);
    $q->execute();

A) is this enough to sanitize information? b) when I put HTML tags in there like <b>name</b> it actually shows up in bold on my site! Is there an option where I can have PDO strip out all HTML?

3

3 Answers

2
votes

Looks reasonably sound. I would suggest using POST instead of GET for destructive / manipulative operations though. You're far less likely to suffer from CSRF attacks if you stick to POST data though it does not make you totally immune.

If you do not actually want users to enter HTML into the name field, don't worry about filtering data on the way into the database. Escape it on the way out via htmlspecialchars() or htmlentities().

I've always stood by the idea that data should go into the database as raw as possible.

Edit: Almost forgot, make sure the expected values in $_GET / $_POST actually exist before attempting to use them, eg

if (isset($_POST['name'])) {
    // now use it
1
votes

A) Read manual:

The parameters to prepared statements don't need to be quoted; the driver automatically handles this. If an application exclusively uses prepared statements, the developer can be sure that no SQL injection will occur (however, if other portions of the query are being built up with unescaped input, SQL injection is still possible).

B) Never trust to user's data. Use htmlspecialchars in output.

C) Use $_POST and tokens for queries, which will change any data, to avoid CSRF.

0
votes

Never trust user input! At a minimum, wrap the $_GET['name'] in a sanitizing function, like mysql_real_escape_string() to prevent SQL Injection attacks. And then when you're outputting user-supplied data, make sure to wrap it in htmlspecialchars() to prevent Cross-Site Scripting (XSS) attacks.