where did i did my mistake ? Got a MySQL syntax error in my cms

Question

I am making a simple cms for managing someones site , although when I try the to modify the access level of the user account , it gives a mysql sytax error:-

'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE user_id = 2' at line 5'

Programme has 3 levels of users , 1 = user, 2 = moderator , 3 = administrator.

Here is my code:

<?php
require_once 'db.inc.php';
require_once 'cms_http_functions.inc.php';

$db = mysql_connect(MYSQL_HOST, MYSQL_USER, MYSQL_PASSWORD) or
    die ('Unable to connect. Check your connection parameters.');

mysql_select_db(MYSQL_DB, $db) or die(mysql_error($db));

if (isset($_REQUEST['action'])) {

    switch ($_REQUEST['action']) {
    case 'Login':
        $email = (isset($_POST['email'])) ? $_POST['email'] : '';
        $password = (isset($_POST['password'])) ? $_POST['password'] : '';
        $sql = 'SELECT
                user_id, access_level, name
            FROM
                cms_users
            WHERE
                email = "' . mysql_real_escape_string($email, $db) . '" AND
                password = PASSWORD("' . mysql_real_escape_string($password,
                    $db) . '")';
        $result = mysql_query($sql, $db) or die(mysql_error($db));
            if (mysql_num_rows($result) > 0) {
            $row = mysql_fetch_array($result);
            extract($row);
            session_start();
            $_SESSION['user_id'] = $user_id;
            $_SESSION['access_level'] = $access_level;
            $_SESSION['name'] = $name;
        }
        mysql_free_result($result);
        redirect('cms_index.php');
        break;

    case 'Logout':
        session_start();
        session_unset();
        session_destroy();
        redirect('cms_index.php');
        break;

    case 'Create Account':
        $name = (isset($_POST['name'])) ? $_POST['name'] : '';
        $email = (isset($_POST['email'])) ? $_POST['email'] : '';
        $password_1 = (isset($_POST['password_1'])) ? $_POST['password_1'] : '';
        $password_2 = (isset($_POST['password_2'])) ? $_POST['password_2'] : '';
        $password = ($password_1 == $password_2) ? $password_1 : '';
        if (!empty($name) && !empty($email) && !empty($password)) {
            $sql = 'INSERT INTO cms_users
                    (email, password, name)
                VALUES
                ("' . mysql_real_escape_string($email, $db) . '",
                PASSWORD("' . mysql_real_escape_string($password, $db) . '"), 
                "' . mysql_real_escape_string($name, $db) . '")';
            mysql_query($sql, $db) or die(mysql_error($db));

            session_start();
            $_SESSION['user_id'] = mysql_insert_id($db);
            $_SESSION['access_level'] = 1;
            $_SESSION['name'] = $name;
        }
         redirect('cms_index.php');
        break;
    enter code here
    case 'Modify Account':
        $user_id = (isset($_POST['user_id'])) ? $_POST['user_id'] : '';
        $email = (isset($_POST['email'])) ? $_POST['email'] : '';
        $name = (isset($_POST['name'])) ? $_POST['name'] : '';
        $access_level = (isset($_POST['access_level'])) ? $_POST['access_level']
            : '';
        if (!empty($user_id) && !empty($name) && !empty($email) &&
             !empty($access_level) && !empty($user_id)) {
            $sql = 'UPDATE cms_users SET
                    email = "' . mysql_real_escape_string($email, $db) . '",
                    name = "' . mysql_real_escape_string($name, $db) . '",
                    access_level = "' . mysql_real_escape_string($access_level,
                        $db) . '",
                WHERE
                    user_id = ' . $user_id;
            mysql_query($sql, $db) or die(mysql_error($db));
        }
        redirect('cms_admin.php');
        break;

    case 'Send my reminder!':
        $email = (isset($_POST['email'])) ? $_POST['email'] : '';
        if (!empty($email)) {
            $sql = 'SELECT email FROM cms_users WHERE email="' .
                mysql_real_escape_string($email, $db) . '"';
            $result = mysql_query($sql, $db) or die(mysql_error($db));
            if (mysql_num_rows($result) > 0) {
                $password = strtoupper(substr(sha1(time()), rand(0, 32), 8));
                $subject = 'Comic site password reset';
                $body = 'Looks like you forgot your password, eh? No worries. ' . 
                    'We\'ve reset it for you!' . "\n\n";
                $body .= 'Your new password is: ' . $password;
                mail($email, $subject, $body);
            }
            mysql_free_result($result);
        }
        redirect('cms_login.php');
        break;

    case 'Change my info':
        session_start();
        $email = (isset($_POST['email'])) ? $_POST['email'] : '';
        $name = (isset($_POST['name'])) ? $_POST['name'] : '';
        if (!empty($name) && !empty($email) && !empty($_SESSION['user_id']))
        {
            $sql = 'UPDATE cms_users SET
                    email = "' . mysql_real_escape_string($email, $db) . '",
                    name = "' . mysql_real_escape_string($name, $db) . '",
                WHERE
                    user_id = ' . $_SESSION['user_id'];
            mysql_query($sql, $db) or die(mysql_error($db));
        }
        redirect('cms_cpanel.php');
        break;
    default:
        redirect('cms_index.php');
    }
} else {
    redirect('cms_index.php');
}
?>

I can't seem to find any error in the code. Please Help.

Print out the `$sql`; don't use `mysql_` functions, read up on [SQL injection](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php). — , Sep 23 '13 at 02:54
This question appears to be off-topic because it is about finding a comma — Wooble, Sep 23 '13 at 02:56
@Wooble, I'm pretty certain that doesn't make it off-topic, otherwise there's a massive quantity of runtime errors in _any_ code which would be disallowed on SO, such as why `for (i = 0; i < 10; j++)` never ends. — paxdiablo, Sep 23 '13 at 03:03

Boann · Answer 1 · 2013-09-23T03:01:26.290

In the 'Modify Account' case, you have an extra comma on one line:

                access_level = "' . mysql_real_escape_string($access_level,
                    $db) . '",
                             ^ here

But I beg of you, don't use the mysql_ functions in new code. They're messy, obsolete, and officially deprecated. Learn PHP's PDO for database access. Once you get used to it you'll find it's easier, neater, and safer.

paxdiablo · Answer 2 · 2013-09-23T03:05:52.547

The snippet:

'access_level = "' . mysql_real_escape_string($access_level, $db) . '", WHERE...'

(it's easier to see on one line) has a comma before the where clause.

Get rid of it. It's okay to have that comma when you're setting another column but not just before the where.

Keep in mind that, in 90% of cases, these problems are easy to detect if you simply output your SQL strings before executing them (during debugging, not in production).

Also, you need to learn how to use parameterised queries, both for readability and protection against potential security holes (SQL injections).

where did i did my mistake ? Got a MySQL syntax error in my cms

2 Answers2