What is the problem with this php script?

Question

I keep getting error 1064 in this line:

$sqlquery = "INSERT INTO user 
               (username, password, email, key) 
             VALUES 
                ('".$_POST["username"]."','".$_POST["password"]."','".$_POST["email"]."','".$activation."')";`

I guess you should read this: http://en.wikipedia.org/wiki/SQL_injection — jantimon, May 06 '11 at 14:33
Oh goodness..$_POST values in the SQL. You should really look into escaping the values properly or using prepared statements. — barfoon, May 06 '11 at 14:36
Could you provide the displayed SQL error? Thi will help a lot debugging a problem with a sql statement when the database architecture is unknown. — Bearwulf, May 06 '11 at 14:37

Rudi Visser · Accepted Answer · 2011-05-06T14:45:23.890

12

key is a reserved word which you're using in your query, this must be escaped with backticks. Reserved word error is 1064.

You should also consider learning some security theory particularly with regards to using unescaped values in a query (straight from a user).

The below code is both secure and fixed:

$username = mysql_real_escape_string($_POST['username']);
$password = mysql_real_escape_string($_POST['password']);
$email = mysql_real_escape_string($_POST['email']);
$sqlquery = "INSERT INTO `user` (`username`, `password`, `email`, `key`) VALUES ('{$username}','{$password}','{$email}','{$activation}')";

A simple rule when it comes to queries (well, anything) is to never trust user input. By using mysql_real_escape_string you're escaping the variables so that they're safe for insertion into the database. Without it, you could allow the user to run any query that they wanted to.

For future reference, here is a complete list of MySQL Reserved Words.

edited May 06 '11 at 14:45

answered May 06 '11 at 14:37

Rudi Visser

21,350
5
71
97

thx! This is just a simple test, no security required ;) – Urinprobe May 06 '11 at 14:38
Fair enough, then just enquote `key` and job done :-) – Rudi Visser May 06 '11 at 14:39
+1 For explaining the security problems. Even if it's just a test, you should get used to always filtering user input (anything that comes from the outside) the right way. – kapa May 06 '11 at 14:46
Good catch on use of reserved word. – AJ. May 06 '11 at 14:47

score 2 · Answer 2 · answered May 06 '11 at 14:34

2

MySQL error 1064 generally means a SQL syntax error. Take a look at your SQL statement to make sure it's valid.

A good way to debug those kinds of errors is to print out the SQL, then try to execute it manually in MySQL.

answered May 06 '11 at 14:34

Mark Biek

146,731
54
156
201

Jip that's true and as long as we don't know the content of each variable it's though to post the exact problem :) – tim May 06 '11 at 14:35
1064You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'key) VALUES ('Simon','abc123','mail@tld.com','58d2f92539a5492fe6ef' at line 1 – Urinprobe May 06 '11 at 14:35

score 1 · Answer 3 · edited May 06 '11 at 14:47

1

Try surrounding each variable $var with mysql_real_escape_string(), such as

instead of $_POST["password"] use mysql_real_escape_string($_POST["password"])!

edited May 06 '11 at 14:47

kapa

77,694
21
158
175

answered May 06 '11 at 14:34

tim

9,896
20
81
137

This is not the primary issue with the query. – Rudi Visser May 06 '11 at 14:37
@rudi I'd say it's one of the primary issues with the query. – kapa May 06 '11 at 14:48

score 1 · Answer 4 · answered May 06 '11 at 14:41

Do you still get errors if you use this instead:

$query = sprintf("INSERT INTO user 
                    (username, password, email, `key`) 
                  VALUES 
                    ('%s','%s','%s','%s')",
                  mysql_real_escape_string($_POST["username"]),
                  mysql_real_escape_string($_POST["password"]),
                  mysql_real_escape_string($_POST["email"]),
                  mysql_real_escape_string($_POST["activation"]));

$result = mysql_query($query);

KEY is a MySQL reserved word -- it needs to be enclosed in backticks to escape its use in queries. Backticks are not necessary if not using reserved words...

score 0 · Answer 5 · answered May 06 '11 at 14:35

0

Taking user-defined values directly from the HTTP Request and concatenating them into an SQL query is B-A-D, and likely the source of your syntax error. Make sure you escape all values.

answered May 06 '11 at 14:35

AJ.

27,586
18
84
94

What is the problem with this php script?

5 Answers5