Simple PHP Login - num_rows

Question

I am using mysql_num_rows to check if one row is returned for my user login and if count == 1 then log user in, I get an error though below my code is after that. Any suggestions?

Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in /home/web42001spring09/rcoughlin/public_html/process-login.php on line 13

<?php
// include database info
include("config.php");

if(isset($_POST["submit"])){
    // get data from form
    $username = $_POST["user"];
    $password = $_POST["pass"];

    $query  = "SELECT username,password,id FROM login WHERE username=".$username." AND password=".$password." LIMIT 1";
    $result = mysql_query($query);

    $count  = mysql_num_rows($result);

    // if 1 then login them in set cookie and redirect
    if($count==1){
     setcookie("password", "".$password."", time()+3600);
     setcookie("username", "".$username."", time()+3600);
     header("Location:admin.php");
    }else{
     echo "Wrong Username or password combination";
    }
}else{
    echo "Must be submitted via form.";
}

Not sure why the code is drawing that issue? I have used this method before.

Thanks,

Ryan

Answer 1

Looks like you might want to do 2 things:

1. Sanitise your input - passing user-submitted data straight into a database query is a recipe for disaster (see mysql_real_escape_string(string $unescaped_string)).
2. Put quotes around literals in database queries (i.e. username ='".$username."')

The error message you're getting is due to the fact that the MySQL result object ($result) is not valid. Try calling mysql_error() to see what error message MySQL returns.

Answer 2

The query is invalid (the $result == false)

Line:

$query  = "SELECT username,password,id FROM login WHERE username=".$username." AND password=".$password." LIMIT 1";

Should be replaced by:

$query  = "SELECT username,password,id FROM login WHERE username='".mysql_escape_string($username)."' AND password='".mysql_escape_string$password)."' LIMIT 1";

The PHP mysql_query() function doesn't give errors by default. Using a function that shows sql errors allow you to spot these errors easily.

function my_query($sql) {
  $result = mysql_query($sql);
  if ($result === false) {
    trigger_error('['.mysql_errno().'] '.mysql_error(), E_USER_WARNING);
  }
  return $result;
}

Right now the username and password are injected directly in the sql string.
What you want is password = "secret", now the query contains password = secret
Mysql error "unknown column sercet"

PS: Using a "LIMIT 1" here is a sign that there are several users (ids) using the same username password combination. (Not recommended)

Answer 3

Try:

  $row = mysql_fetch_row($result);
  if ($row) { // success

Assuming you'd probably want to fetch some columns along with the authentication check (e.g. real name, last login etc.)

Answer 4

You're not quoting your strings in the query, so your SQL statement is invalid.

$query  = "SELECT username,password,id FROM login WHERE username='" . mysql_escape_string($username) . "' AND password = '" . mysql_escape_string($password) . "' LIMIT 1";
$result = mysql_query($query) or die(mysql_error());

You need to add quotes AND use mysql_escape_string or mysql_real_escape_string to prevent SQL injection attacks.

Answer 5

cuz password and login are strings. And u need to change the sql:

$query="SELECT username,password,id FROM login WHERE username='".$username."' AND password='".$password."' LIMIT 1"

Answer 6

Firstly, check for errors...

The "supplied argument is not a valid MySQL result resource" because there was an error in your SQL, but you haven't made life easy for yourself by ignoring the failed query. Use mysql_error to get the error message.

Secondly, properly escape strings in SQL...

Once you see the error, you'll see you missed some quotes in your query, but you must also escape the strings you put in a query, otherwise you're vulnerable to SQL injection attacks...

$query  = "SELECT username,password,id FROM login ".
    "WHERE username='".mysql_real_escape_string($username)."' ".
    "AND password='".mysql_real_escape_string($password)."' LIMIT 1";

$result = mysql_query($query);
if ($result)
{

    $count  = mysql_num_rows($result);

    // if 1 then login them in set cookie and redirect
    if($count==1){
     setcookie("password", "".$password."", time()+3600);
     setcookie("username", "".$username."", time()+3600);
     header("Location:admin.php");
    }else{
     echo "Wrong Username or password combination";
    }
}
else
{
    echo "Error:".mysql_error()."<br>;
}

Always use mysql_real_escape_string when building a query, or use a wrapper class library which does it for you with parameterised queries, like PDO or ADODb

Finally, a word on those cookies...

Also, logging someone in by giving them a cookie with the username and password isn't a terribly good way to implement a login. Aside from transmitting the password in the clear with every request, it's highly vulnerable to a cookie theft attempt. Given your naive approach to SQL security, it's likely you'll also be leaving yourself vulnerable to XSS attacks making it easy for someone to collect those cookies :)

Answer 7

Do never store user authentication data in a cookie!