MySQL / PHP - Is this a good and secure way to escape in querys?

Question

I've finally made what I think is a good, secure and fast way to execute a query, but I want to be completely sure before I implement it all over the site.

My code:

$email = $_POST['email'];
$displayName = $_POST['displayName'];
$pass = $_POST['pass1'];

if($stmt = $link -> prepare("INSERT INTO profiles (email, displayName, password) VALUES (?, ?, md5(?))")) {

        /* Bind parameters
            s - string, b - boolean, i - int, etc */
        $stmt -> bind_param("sss", $email, $displayName, $pass);

        /* Execute it */
        $stmt -> execute();

        echo "You are now registered.<br />";
        echo "<a href=\"login.php\">Login</a>";


        /* Close statement */
        $stmt -> close();
    }

BTW, what does stmt mean/stand for?

EDIT, NEW CODE:

    /* Create a prepared statement */

    $stmt = $link -> prepare("INSERT INTO profiles (email, displayName, password,
    dateRegistered) VALUES (?, ?, md5(?), NOW())");

    if ( false===$stmt ) {
      die('prepare() failed: ' . htmlspecialchars($link->error));
    }

    $rc = $stmt -> bind_param("sss", $email, $displayName, $pass);
    if ( false===$rc ) {
      die('bind_param() failed: ' . htmlspecialchars($stmt->error));
    }

    /* Execute it */
    $rc = $stmt->execute();
    if ( false===$rc ) {
      die('execute() failed: ' . htmlspecialchars($stmt->error));
    }

    echo "You are now registered.<br />";
    echo "<a href=\"login.php\">Login</a>";


    /* Close statement */
    $stmt -> close();

you should not use md5() for passwords, it is not secure anymore. (and has never been secure without the use of a salt). You should *always* do hashing on the PHP side, otherwise the password could/will end up in the MySQL log files. — Jacco, Jul 16 '12 at 09:54

Mitya · Accepted Answer · 2012-07-13T22:57:18.943

2

Yes - it's a prepared statement which pretty much avoids risk of SQL injection, which is the main purpose behind prepared statements.

The only downside is they can be troublesome when used in utilities that have to work with different queries, with a dynamic number of fields, say. You can use reflection to get round this, though.

A few pointers, though:

md5 for passwords? Probably not the safest option. Consider using an encryption salt (lots of stuff on this if you Google it)
you seem to be taking data straight from the $_POST superglobal without checks or sanisation, but I guess that was just to keep the length of the code snippet down for this SO question. Never insert straight from input to query - there should be a phase of validation/escaping/encoding etc.
you don't seem to be checking that the execution of the statement was successful - you assume it was and then proceed to feedback. Check for errors first.

edited Jul 13 '12 at 22:57

answered Jul 13 '12 at 22:45

Mitya

33,629
9
60
107

Isn't he still vulnerable to XSS attacks? – Paul Dessert Jul 13 '12 at 22:46
Thanks for your answer! md5 is just for now, I will improve the password security later on. I know that passwords is a big thing but I will get into it. When you say checks or sanisation for the $_POST I guess you mean things like, if(isset($_POST['email'])) etc. Regarding if the statement was successful or not, I've just started with prepared statements so I don't know the syntax for it. Could you maybe give me a link for some short information about it? – Oskar Persson Jul 13 '12 at 22:47
@Paul - well I think my second bullet point covers that to a point, as sanisation and validation of data would cover this. For the benefit of the OP, [here's more info on this](http://stackoverflow.com/questions/568995/best-way-to-defend-against-mysql-injection-and-cross-site-scripting). – Mitya Jul 13 '12 at 22:50
1

@Utkanos Thanks for the link. I brought it up because I'm not 100% sure. I was under the assumption, until yesterday, that PDO was effective agains XSS as well – Paul Dessert Jul 13 '12 at 22:53
@Oskwish - have a look at the PHP doc's for [prepared statements](http://www.php.net/manual/en/class.mysqli-stmt.php) and for MySQLI in general. `execute()` returns `false` on failure, so in the least you could do a check along the lines of `if (!$stmt->execute()) $error = 'something went wrong';` Re: sanitisation, yes - checking the values actually exists, and checking them against / cleaning them of any unwelcome content. Essentially we're talking validation. – Mitya Jul 14 '12 at 10:24

MySQL / PHP - Is this a good and secure way to escape in querys?

1 Answers1