what's wrong with this sql query?

Question

Okay I have two variables in PHP

$username;
$password;

which are initialized to the data retrieved from $_POST variable :)

I have this SQL query

$sql = "SELECT * FROM users WHERE username = '" . $username . "' AND password = '" . $password . "')";

But this doesn't works and returns me nothing :(

Can you instruct me into the right direction. Please?

Answer 1

eeek! sql injection for one!

EDIT: http://stackoverflow.com/questions/84556/whats-your-favorite-programmer-cartoon/84629#84629

Answer 2

What's wrong with it?

Everything, unfortunately. In particular it's open to SQL injection attacks.

If that's a verbatim cut&paste, then the reason it's not actually working is a trailing closing bracket. Presumably you're not checking for errors when you call this?

Using the base MySQL API it should be:

$sth = $db->prepare("SELECT COUNT(*) FROM users WHERE username = ? AND password = ?");
$sth->execute($username, $password);
list($count) = $sth->fetchrow();
$authorized = ($count > 0);

or similar (code untested, E&OE, etc...)

Answer 3

The query has a closing parenthesis on the end for no reason, it won't work.

Answer 4

Why is there a stray ) at the end of your query? It shouldn't be there.

Oh, and thirded on SQL injection. BAD.

Answer 5

You seem to have an excess closing parenthesis at the end of your query string.

[Edit] - for those screaming SQL injection attacks: we don't know what the user has done with their variables before using them in the query. How about benefit of doubt? ;-)

Answer 6

First of all, never, ever do it like this. Please read about SQL injection and don't write any SQL until you have understood what it says. Sorry, but this is really essential.

That said, your query contains a closing bracket. That looks like a syntax error. Do you get an error executing it?

Answer 7

Enjoy this:

Answer 8

There's an extra parenthesis on the right hand side of the query.

Also, if you do not sanitize your code properly you're going to be vulnerable to SQL injection. You should really be using parameterized queries, but in lieu of that at least use mysql_real_escape_string() on $username and $password.

Also, as a bit of ghost debugging, it's very possible that your passwords are MD5 hashed in the database, since you should never store them in plain text.

Try:

$username = mysql_real_escape_string($_POST["username"]);
$password = md5($_POST["password"]);

$sql = "SELECT * FROM users WHERE username = '$username' AND password = '$password'";

Answer 9

In addition to all the other problems noted. The Password in the Users table is stored encrypted. Unless you've run the Password through the MySQL password encryptor, you will never see any data from this query as the passwords won't match.

Answer 10

Hey guys thank you that solved the problem.

Well, I have filtered the data already before storing it in $username and $password. By filtering, I mean I have applied the mysql_real_escape_string() function to the input.

I just didn't show the whole script here just to make the answering process easier :)

Anyways thanks again both for the answer and for the advice.