Successful execution despite "error in your SQL syntax" message

Question

My php code is executing successfully despite an error message popping up. The code deletes a row from one of my tables but the following error message is displayed:

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 '1' at line 1

My php code is as follows:

 <?php
$id = $_GET['meeting_id'];
$username = $_GET['username'];

$result = mysql_query("DELETE FROM attendees WHERE meeting_id = '$id' AND username = '$username'")
or die(mysql_error());

if (!mysql_query($result))
  {
        die('Error: ' . mysql_error());
  }
  else 
  {
      echo '<h2>The User Has Been Removed From The Meeting</h2>';

  }
?>

Can anyone see a solution here? thanks

It seems `meeting_id` is an integer type column, thus `$1` without quotes. Second: This code is _highly_ insecure! Validate _any_ input! And last: `echo` the query, to see, what is the result. — KingCrunch, Jan 05 '12 at 20:09
http://php.net/manual/en/security.database.sql-injection.php — Pekka, Jan 05 '12 at 20:10
See [Best way to stop SQL Injection in PHP](http://stackoverflow.com/questions/60174/best-way-to-stop-sql-injection-in-php) to find out how that works. — hakre, Jan 05 '12 at 20:10
I'm assuming your $username isn't `Robert'); DROP TABLE attendees;--` — minboost, Jan 05 '12 at 20:11
identifier is an int, you don't have to use quotes for that. And put some `$id = intval($_GET['meeting_id'])` at least to get rid of malformed input.. — ashein, Jan 05 '12 at 20:11
@KingCrunch, Robert - quotes for int columns are fine; in fact using them makes it possible to use `mysql_real_escape_string()` for int values, while it's useless when quotes are not used. — Pekka, Jan 05 '12 at 20:11
@minboost I am very interested in how would I fix (meaning that: I want to see it in action, purposly) something like: `SELECT * FROM `table` WHERE `smth` = 'rob'; DELETE FROM `user_auth` WHERE 1 OR '1'` that came exactly from this: `$a = '"; DELETE FROM `user_auth` WHERE 1 OR "1'; $b = 'SELECT * FROM `values` = "'.$a.'"'; mysql_query($b);` — khael, Jan 05 '12 at 20:24
I want to find all the books where people are taught to do things like this **and burn them**. SQL injection is not a joke. — tadman, Jan 05 '12 at 20:34
@tadman, well, I just tried the above code, and my ancient `mysql_query` function spits me back with an sql error message, although it will actually work in sql. Check it and tell me if I am not right. — khael, Jan 05 '12 at 20:36
`mysql_query` can only handle one query at a time. You can't use the `;` delimiter which is a function of the command-line client only. — tadman, Jan 05 '12 at 20:45
@tadman, exactly! why is everybody so wry about such kind of sql injection, sql injection is not that simple as it seems. I am sick of all those out there that first thing they do is to notice that the code is open to sql injection instead of answering the question and then try to teach good practices. — khael, Jan 05 '12 at 20:54
@khael SQL injection is not a joke, it is very serious. People who post code that is vulnerable to injection **should** get raked over the coals. If you don't know how to write a SQL query properly, that's your **first problem**. The second problem is whatever you were asking. If you don't know how to escape using the `?` placeholder or the various `escape` functions, figure it out **right now**. You are a dangerous liability to any organization you work for if you don't know how to do this properly. — tadman, Jan 05 '12 at 22:43
well, liability or not, anyone untill now have failed in finding an sql attack possibility in my solutions. And I have been through a lot of sql coding these days. The fact is that even the code is not sql proof, the real problem does not stand there. — khael, Jan 05 '12 at 22:48
@khael My post was a joke, based on the XKCD comic: http://xkcd.com/327/ — minboost, Jan 06 '12 at 00:35

Pekka · Answer 1 · 2012-01-05T20:18:01.760

11

You are running mysql_query() twice; once with the (correct) query and a second time

 if (!mysql_query($result))

with the result of the previous query. This will lead to an error.

You probably want

 if (!mysql_fetch_object($result))

or something similar.

Also, as pointed out in the comments section, your PHP code is vulnerable to SQL injection, something you should fix.

edited Jan 05 '12 at 20:18

answered Jan 05 '12 at 20:10

Pekka

442,112
142
972
1,088

1

You may also want to mention the vulnerabilities in his code and link him here: http://www.tizag.com/mysqlTutorial/mysql-php-sql-injection.php or tell him about PDO – Paul Jan 05 '12 at 20:12
@user that's always a good point. I already did that in the comments section above, but I'll add it to the answer – Pekka Jan 05 '12 at 20:17

score 1 · Answer 2 · answered Jan 05 '12 at 20:10

1

You call mysql_query() on your query, and then you call it again on the result of the query. this is where you problem is. remove that second mysql_query() call.

answered Jan 05 '12 at 20:10

Niet the Dark Absol

320,036
81
464
592

score 1 · Accepted Answer · answered Jan 05 '12 at 20:11

You are executing the query twice, once on the result set. Try this:

$result = mysql_query("DELETE FROM attendees WHERE meeting_id = '$id' AND username = '$username'");

if (!$result)
  {
        die('Error: ' . mysql_error());
  }
  else 
  {
      echo '<h2>The User Has Been Removed From The Meeting</h2>';

  }

BTW: your code is prone to SQL injections

Successful execution despite "error in your SQL syntax" message

3 Answers3