views:

411

answers:

5

Hi all,

In accessing my database, I have the user fill out a form, and in the target page, the posted values are used in the resulting MySQL query.

$query = mysql_query("SELECT pass FROM database WHERE user='$_POST[user]'");

However, for some reason or another, MySQL doesn't like my using a $_POST variable in the command, and it only works if I define (for example) $user = $_POST['user'];, and then put $user directly in the SQL command.

On the other hand, I can use $_POST values in INSERT statements where specific column names are not required:

$query = mysql_query("INSERT INTO database VALUES ('foo', 'bar', '$_POST[user]'");

If I try an INSERT statement where attributes are defined (e.g. user='foo'), then the same problem appears.

What am I doing wrong in my SQL query that causes the command to error out when run, but works with the specific method of formatting an INSERT command?

Hopefully, it's not "tough luck, looks like you have to assign all of your posted values". Heh.

+6  A: 

First of, watch out for SQL Injections!

Now, to answer your question try doing this instead:

$query = mysql_query("SELECT `pass` FROM `database` WHERE `user` LIKE '" . mysql_escape_string($_POST['user']) . "';");

You were doing a couple of things wrong:

  • using the = operator instead of LIKE operator
  • not enclosing the value in the SQL query with '
  • not enclosing the user index in the $_POST array with '

PS: You should use mysql_real_escape_string() instead of mysql_escape_string()!

Alix Axel
Just asking, why use like, when the string is not meant to contain any % symbols?
Alex JL
@Salsa: For several reasons, see http://dev.mysql.com/doc/refman/5.0/en/string-comparison-functions.html
Alix Axel
Wow, some good hints here, along with all of the other answers. Good warning about the SQL injection, as well... must revamp my code ASAP.
Julian H. Lam
Why do you use `mysql_escape_string` in the code sample and then tell OP not to use it?
Matchu
@Matchu: Because the `mysql_real_escape_string()` needs the `$link_identifier` argument, and I don't know which variable the OP is using for the database connection.
Alix Axel
@julian.lam: No problem, glad I could help. Also read my comment answering Matchu.
Alix Axel
Back when I used the mysql functions instead of PDO, I never provided the link identifier, and it always worked. In fact, the PHP docs say it's optional: http://us.php.net/mysql_real_escape_string
Matchu
@Matchu: I've always had trouble with that, even today I answered a question where the problem was the $link_identifier... Anyway, the tip is there - the OP should use it.
Alix Axel
It appears, after reading the linked doc page, that the main reason to use `LIKE` rather than `=`, is that `=` ignores trailing whitespace, so the match is potentially inexact. Is there anything else I'm missing?
Frank Farmer
@Frank Farmer: Collations are also involved. As a general rule I consider a best-practice using `LIKE` instead of `=` whenever string comparisons are needed.
Alix Axel
+2  A: 

You're simply inserting a variable into a string, so it shouldn't matter which command you're putting it into.

There are a few issues to point out.

One, you might want to use the {} format for array variables. You don't use quotes around the arrray key names in this format.

$query = mysql_query("SELECT pass FROM database WHERE user='{$_POST[user]}'")

Two, you'd never want to make a query like that because you are open to sql injection holes. Consider, what if $_POST['user'] was "cow';drop table database;--"?

You must either run mysql_real_escape_string on the POST input before putting it into your query, or check out using PHP PDO with prepared statements.

One way to do format your string which provides a bit of structure is to use sprintf.

$query=mysql_query(sprintf("SELECT pass FROM database WHERE user='%s'",mysql_real_escape_string($_POST['user'])));
Alex JL
+1 for starting off with the reason the OP's variable reference wasn't being parsed correctly http://www.php.net/manual/en/language.types.string.php#language.types.string.parsing
Frank Farmer
A: 

Try

$query = mysql_query("SELECT pass FROM database WHERE user=" . mysql_real_escape_string($_POST['user']));

and

$query = mysql_query("INSERT INTO database VALUES ('foo', 'bar', " . mysql_real_escape_string($_POST['user']) . ")");

Its always a good idea to sanitize anything received through $_GET or $_POST

leChuck
+1  A: 

Why not check and see what mysql_error() has to say about it? If your query is invalid, mysql_error() will return a nice blob of text telling you exactly what went wrong.

As for MySQL not liking the POST var if you insert it directly for some runs, but not others, then you should make sure you're using consistent data and setups for each test. If some test are done using a GET, then your POST vars will be empty. If you're using different user names for each test, then see if what's consistent between the ones that fail.

And as mentioned above, read up about SQL injection and how your query is just begging to be subverted by a malicious user.

Marc B
+1  A: 
  1. Use PDO - it provides much better API to communicate with DB.
  2. If you're using mysql_*() functions always remember to filter (mysql_real_escape_string()) any data that comes from untrusted source (like user)
  3. Pay more attention to how your code looks like. Just compare the following listings:

    $query = mysql_query("INSERT INTO database VALUES ('foo', 'bar', " . mysql_real_escape_string($_POST['user']) . ", " . mysql_real_escape_string($_POST['user']) . ", " . mysql_real_escape_string($_POST['user']) . ", " . mysql_real_escape_string($_POST['user']) . ")");
    
    
    $query = sprinf('INSERT INTO database VALUES ("foo", "bar", "%s", "%s", "%s")',
    mysql_real_escape(...), ...);
    

    Do I have to explain which one is better to read, modify or understand?

Crozin
I second PDO, learn it, use it, love it. Unless your working within a good Framework, then use their's.
Clutch
Agreed, PDO is far superior to the basic mysql_* functions.
Alex JL
(Several months later) - definitely using PDO now.
Julian H. Lam