[SOLVED] Need help with login script

[wikedawsum]

Hello all. I have a login script that I'm trying to make more secure. It works fine, but my main concern right now is this: If a user logs in with a bogus username and password, it will log them into the members area. They aren't able to view any sensitive information because there is nothing setup with that username, but I don't want them to even be able to see the member's area if they aren't a registered user. I've tried several different things and cannot figure out why my script is allowing this. Please be aware that I am new to PHP and am still learning.

 

My member.php page that my login form is pointing to:

 

<?php

// include function files for this application
require_once('users_fns.php'); 
session_start();

if (!isset($_SESSION['valid_user'])){
  //create short variable names
  $username = $_POST['username'];
  $passwd = $_POST['passwd'];

$conn = mysql_connect("******", "******", "******") 
   or die($msg_no_connect);
   mysql_select_db("users") 
   or die(mysql_error());

  // Run query
  $sql = "SELECT * FROM user WHERE username='$username' and passwd=sha1('$passwd')";
  $r = mysql_query($sql);
  if(!$r){
     $err=mysql_error();
     echo $err;
     exit();
  }

  if(mysql_num_rows($r) > 0){
     echo "no such login in the system. please try again.";
     exit();
  }
  else{
  $_SESSION['valid_user'] = $username;
  }
}


do_html_header('');

display_user_menu('');

check_valid_user('');

?>

HTML for member page here.....
  
<?php
do_html_footer('');
?>

 

This is my check_valid_user function:

 

function check_valid_user()
// see if somebody is logged in and notify them if not
{
  if (isset($_SESSION['valid_user']))
  {
  	  echo '';
  }
  else
  {
     // they are not logged in
 echo '<br />';
     echo 'You are not logged in.<br />';
     exit;
  }  
}

 

Any help or a point in the right direction would be greatly appreciated. Thanks in advance!

[chronister]

Since you are looking for exactly one user here. I would change the mysql_num_rows line to ......

 

 

$sql = "SELECT * FROM user WHERE username='$username' and passwd=sha1('$passwd')";
  $r = mysql_query($sql);
  if(!$r){
     $err=mysql_error();
     echo $err;
     exit();
  }

  if(mysql_num_rows($r) < 1 ){                         // if 0 results are returned, then print error message 
     echo "no such login in the system. please try again.";
     exit();
  }
  else{                                                        // this condition will be ran if the results are 1 or more
  $_SESSION['valid_user'] = $username;
  }
}

 

Try that and let us know if it works.

[wikedawsum]

chronister,

 

That prevented bogus users from logging in, but it also prevents registered users from logging in as well.

 

Thanks,

wikedawsum

[chronister]

Then echo your query, Sounds like it is not returning any rows.

 

$sql = "SELECT * FROM user WHERE username='$username' and passwd=sha1('$passwd')";
  $r = mysql_query($sql);
  if(!$r){
     $err=mysql_error();
     echo $err;
     exit();
  }
echo $sql.'<br>'; // make sure that your query is populating the username and password properly

echo mysql_num_rows($r);  // echo the num of results returned to verify that its finding the user.


  if(mysql_num_rows($r) < 1 ){                         // if 0 results are returned, then print error message 
     echo "no such login in the system. please try again.";
     exit();
  }
  else{                                                        // this condition will be ran if the results are 1 or more
  $_SESSION['valid_user'] = $username;
  }
}

 

 

If those 2 items are populating correctly then we will have to look elsewhere, but I think one of these 2 items are the problem.

 

[wikedawsum]

These are the results:

 

SELECT * FROM user WHERE username='test' and passwd=sha1('*****')

0no such login in the system. please try again.

[chronister]

There is your problem..... your password is being parsed as **********  unless you did that for "security"

 

0 rows are being returned...so your query is not right.

[chronister]

I missed this the first reply... sorry

 

 

change the query to this

 

$passwd=sha1($password);

 

$sql = "SELECT * FROM user WHERE username='username' and passwd='$passwd' ";

 

 

It returns something like this

 

 

SELECT * FROM user WHERE username='username' and passwd=5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8

 

[wikedawsum]

Yes, I just did that for security. Sorry I didn't mention that. It returned the correct the password.

 

I'm not sure why the query would be incorrect. I have a view_user page in my admin section that queries the same database and table and it returns the results with no problems. Very puzzling. Thank you for your help, chronister. I appreciate it.

 

 

[chronister]

I may be wrong, but it seems that sha1() cannot be used inside the query.

 

When I tried it on my server the way you have it written the sha1('$passwd') shows through in the query. When you run the passwrod through the sha1() function outside of the query and then use the variable in the query it works.

 

 

 

[wikedawsum]

That worked perfectly!

 

My new member.php file is:

 

<?php

// include function files for this application
require_once('users_fns.php'); 
session_start();

if (!isset($_SESSION['valid_user'])){
  //create short variable names
  $username = $_POST['username'];
  $passwd = $_POST['passwd'];
  $passwd = sha1($passwd);

$conn = mysql_connect("******", "******", "******") 
   or die($msg_no_connect);
   mysql_select_db("users") 
   or die(mysql_error());

  // Run query
  $sql = "SELECT * FROM user WHERE username='$username'";
  $r = mysql_query($sql);
  if(!$r){
     $err=mysql_error();
     echo $err;
     exit();
  }

  if(mysql_num_rows($r) < 1){
     echo "no such login in the system. please try again.";
     exit();
  }
  else{
  $_SESSION['valid_user'] = $username;
  }
}


do_html_header('');

display_user_menu('');

check_valid_user('');

?>

 

Tested it with both bogus and registered users and got the results I wanted. Thanks so much for your help!!!

[chronister]

No Problem...

 

try to get in the habit of a troubleshooting routine.

 

When things don't work the way I want the first things I do is start at the top and ensure that each piece is doing what I want it to do.

 

echo important variables to ensure that they have values you expect, echo queries to ensure that they are producing a valid query. I will take my query, echo it then copy and paste into phpmyadmin and see if it runs that way.

 

If variables and queries look good, then I will generally echo the mysql_num_rows() part to ensure that the query is returning results.

 

If all these things are in order, then I will start in my if/else logic and echo things inside the statements like echo 'if I can read this then Blah Blah is true'.

 

Its just a matter of starting from the beginning and ruling out items until you find the error.

 

 

[wikedawsum]

Those are great tips. I will definitely keep them in mind as I continue learning. Thanks again!