SQL Query Problem

[gavicus]

I'm having problems with my login script. I use the code:

$connection=mysql_connect($server,$username,$passw ord);
$db=mysql_select_db($database,$connection);
$myusername=$_POST['myusername'];
$mypassword=$_POST['mypassword'];

$q = "SELECT * FROM users WHERE username='$myusername' and password='$mypassword';";
$result = mysql_query($q);
$count = mysql_num_rows($result);

...but regardless of my form entry, I get the error:

Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in /home/gavicus/public_html/checklogin.php on line 22

...where line 22 is the call to mysql_num_rows.

Am I doing anything obviously stupid? I've wondered if x10's SQL server is having problems because everything is slow since I started yesterday, but that only makes it impossible for a newby like me to know if I'm doing something wrong.

[garrettroyce]

The first thing I'm going to mention is it's a very very very very very very very bad practice to not sanitize input directly from a user. I could use

' OR 1=1

as my username, which means your query will ALWAYS return a result, even if I'm not a user of your site. See what I mean :P

You also could use some error checking throughout.

$connection=mysql_connect($server,$username,$password) or exit(mysql_error()); //password was passw ord before. now if the connection fails, it will say why.
$db=mysql_select_db($database,$connection) or exit(mysql_error()); //if db select fails, say why
if (!isset($_POST['myusername'], $_POST['mypassword'])) {
     exit('Missing user name or password');
}
$myusername = addslashes($myusername);
$mypassword = addslashes($mypassword);

$q = "SELECT * FROM users WHERE username='$myusername' and password='$mypassword'"; //don't put semicolons in your queries
$result = mysql_query($q);
if ($result === false) {
     exit(mysql_error()); // if the query fails, say why
}
$count = mysql_num_rows($result);

[gavicus]

I use stripslashes and mysql_real_escape_string on both username and password, but I trimmed them out of my message as I don't think they're the problem. Thanks for the response, though. I couldn't agree more.

Oh, I also removed the semicolon from the query string, but got the same results. Why don't you use semicolons in your queries? Isn't that standard SQL?

[garrettroyce]

You use semicolons if you're sending a group of queries. I don't think it's the problem, I just take them out because they don't help the query. Did you put the error checking in? That should tell you what's wrong.

[gavicus]

Ok, I found the problem--it was in my SQL query. Well, like I said, I'm new at this. Still, garrettroyce, how do you put in error checking?

[garrettroyce]

That's the lines I added in my code. You need to check if the connection is successful before changing databases. You need to check if the database change is successful before querying it. You have to check if the query is successful before using the result.

[misson]

Error messages from mysql_error() are going to be too technical and potentially contain sensitive information. They won't contain information the average user can use, and may contain information a malicious user can use. If you need to see the full error, log it with error_log() so that only a site admin can see all the information.

If you use addslashes, your script is still vulnerabel to SQL injection because it doesn't take into account multibyte strings or the syntax used on the DBMS. Prepared statements (via mysqli_prepare or PDO::prepare) are the most secure way of preventing SQL injection.

Does the password column contain plaintext passwords? A much more secure approach is to store a salted hash. There's still the issue of how you'll transmit authentication information if you're not using HTTPS.