PHP + MySQL SQL Syntax error

Question

I have the following code

$id_post = mysql_real_escape_string($_POST['id']);
$forumid = (int)mysql_real_escape_string($_POST['forumid']);
$message = mysql_real_escape_string($_POST['message']);

mysql_query("UPDATE forum_reactions SET message = ".$message." WHERE id = ".$id_post." ");

message is the TEXT column

It gives this error

You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'staat niet in het woordfilter lol WHERE id = 39' at line 1

@Barranka OP is escaping $_POST['message'] and storing it in $message, not passing in un-sanitized user data. — Chris Rasco, Sep 12 '13 at 21:17
I guess Barranka missed the first three lines of `mysql_real_escape_string`. In other news, you should really be using PDO or MySQLi. MySQL is deprecated. — Connor Peet, Sep 12 '13 at 21:18
Yeah, no SQL injection possible here? XSS maybe, but not SQL. And I'm using mysql_ because the website where I'm building it for already uses it. — Calvin, Sep 12 '13 at 21:19
So if the `$message` variable has something like `''; drop table forum_reactions; --` this would work just fine? http://stackoverflow.com/questions/332365/how-does-the-sql-injection-from-the-bobby-tables-xkcd-comic-work — Barranka, Sep 12 '13 at 21:20
@Barranka, that would be correctly escaped by `mysql_real_escape_string`, and would be stored as that string literal. — halfer, Sep 12 '13 at 21:23
Also, if you cast `$_POST['forumid']` to `(int)`, you don't also need to escape it. — halfer, Sep 12 '13 at 21:26
@AndyLester True sir, will be removed, it was reused code :P — Calvin, Sep 12 '13 at 21:26
Utlimately, the key here is that **building SQL statements from outside data is dangerous**. Rather than going through these hoops of figuring out what you have to escape and what you have to cast, stop and put on your Big Programmer Pants and start using prepared statements and bound variables. Here is a fantastic answer that will get you started: http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php You can also check http://bobby-tables.com/php for other examples. — Andy Lester, Sep 12 '13 at 21:30
@AndyLester Thanks I will check that to learn more about this and use it in my projects. — Calvin, Sep 12 '13 at 21:37

score 3 · Accepted Answer · answered Sep 12 '13 at 21:16

3

You aren't encompassing the string for $message so SQL is attempting to use those as keywords, which they aren't. Try this:

$id_post = mysql_real_escape_string($_POST['id']);
$forumid = (int)mysql_real_escape_string($_POST['forumid']);
$message = mysql_real_escape_string($_POST['message']);

mysql_query("UPDATE forum_reactions SET message = '".$message."' WHERE id = ".$id_post." ");

The mysql_* functions are deprecated and you should move to mysqli_* or PDO.

http://php.net/manual/en/function.mysql-query.php

This extension is deprecated as of PHP 5.5.0, and will be removed in the future. Instead, the MySQLi or PDO_MySQL extension should be used. See also MySQL: choosing an API guide and related FAQ for more information. Alternatives to this function include:

mysqli_query() PDO::query()

answered Sep 12 '13 at 21:16

Chris Rasco

2,713
1
18
22

Although this solves the OP issue, this is still prone to SQL Injection attacks. Prepared statements should be used to avoid this problem. – Barranka Sep 12 '13 at 21:17
He should do the same for id shouldn't he? – Jordan Sep 12 '13 at 21:20
@Jordan Only strings need to be enclosed in queries, but it wouldn't hurt. Similarly OP could cast it as an int or use intval() like they did for $forumid. – Chris Rasco Sep 12 '13 at 21:21
@ChrisRasco Because the OP didn't do it for the `id`, I assumed the `id` _was_ a string, silly as that would be. – Jordan Sep 12 '13 at 21:24
Thanks guys, I am not sure why I missed the ' in the first place, I guess I shouldn't do this coding at night haha! – Calvin Sep 12 '13 at 21:24
Yeah I removed the (id) for $id_post to test if that was causing problems, but it wasn't of course. I will put it back – Calvin Sep 12 '13 at 21:25

score 0 · Answer 2 · answered Sep 12 '13 at 21:26

PDO Sample Usage:

<?php

try {

    // config
    $dsn = 'mysql:dbname=testdb;host=127.0.0.1;charset=utf8';
    $username = 'root';
    $password = '';
    $options = array(
        PDO::MYSQL_ATTR_USE_BUFFERED_QUERY => true,
        PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
        PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
    ); // You should always use these options

    // conncect
    $pdo = new PDO($dsn, $username, $password, $options);

    // check posted values
    if (
        !isset($_POST['id'], $_POST['message']) ||
        !is_string($_POST['id']) ||
        !is_string($_POST['message'])
    ) {
        throw new RuntimeException('invalid parameters');
    }

    // SQL execution
    $stmt = $pdo->prepare('UPDATE forum_reactions SET message = ? WHERE id = ?');
    $stmt->execute(array($_POST['message'], $_POST['id']));

    // check result
    if ($stmt->rowCount()) {
        echo 'successfully updated';
    } else {
        echo 'specified ID not found.';
    }

} catch (Exception $e) {

    echo $e->getMessage();

}

PHP + MySQL SQL Syntax error

2 Answers2