Php Logs In With Incorrect User/pass

[devilsvein]

Have a issue which I've put a temporary patch on to prevent unauthorized access. But I still want to know why this is happening

 

Basically my "check" system on login checks the username and password of that typed in. If theres no match it should read out a error message and prevent any more attacks. But what I've found out is....if the passwords "hello123" and you type "hello12" it redirects you to the loggedinpage.....which is wrong.

 

login page extract:

$username = htmlentities($_POST['username']);
$username = mysqli_real_escape_string($mysqli, $username);
$password =mysqli_real_escape_string ($mysqli, $_POST['password']);
$query = mysqli_query($mysqli, "SELECT * FROM Persons WHERE Username = '$username'");
$row = mysqli_fetch_assoc($query);
$numrows = mysqli_num_rows($query);
$dbuser = $row['Username'];
$dbpass = $row['Password'];
$email = $row['Email'];
$_SESSION['login'] = false ;
$salt1 = $dbuser;
$salt2 = $email;
$hash = hash('sha512' , $salt1.$password.$salt2);
$id = $row['PlayerID'];
if( ($username == '') || ($password == '') ) {
$error_string .= '<font color=red>You have left either the username or password field blank!</font>';
$_SESSION['login'] = false ;
}
else if ($numrows == 1)
{
if ($hash == $dbpass)
{
//$error_string .= 'Authentication succeeded';
$_SESSION['login'] = true ;
$_SESSION['username'] = $username;
$_SESSION['email'] = $email;
$_SESSION['ID'] = $id;

header("Location: loggedin.php");


} else {
$error_string .= '<font color=red>Authentication failed</font>';
$_SESSION['login'] = false ;
}



}
else
{
$error_string .= '<font color=red>Authentication failed</font>';
$_SESSION['login'] = false ;

}
}

 

So what I have done is on loggedin.php ive placed now

 

if (empty($_SESSION['username']) || empty($_SESSION['email']) || empty($_SESSION['ID']) || $_SESSION['login'] = false)
{
session_destroy();
header('location: login.php');
die();
}

 

 

So why on earth is login page saying details are correct when there not because if you still type in the wrong password by one letter it redirects you to loggedin.php but as that code is there in loggedin.php it prevents anyone from accessing.

Edited December 30, 2012 by devilsvein

[wotw]

Hey,

 

I have written you something that you could incorporate into your script. I basically wrote this with my eyes closed and I haven't tested it. If you get issues let me know and I can help.

 

You need to add a hidden input into your login form and call it: login & give it a value of 1.

You would also need to implement your password encoding where it says: Do your password encoding.

 

<?php

$case = isset($_POST['login']) ? 'login' : false;
$error = false;
switch($case){

case'login':

 $username = isset($_POST['username']) ? mysqli_real_escape_string($mysqli, $_POST['username']) : false;
 $password = isset($_POST['password']) ? mysqli_real_escape_string($mysqli, $_POST['password']) : false;

 if($username && $password){

  // Do your password encoding here. $password = ?

  $query = mysqli_query($mysqli, "SELECT * FROM Persons WHERE Username = '$username' AND password = '$password'");
  $numrows = mysqli_num_rows($query);

  if($numrows > 0){

   $row = mysqli_fetch_assoc($query);

   // Set sessions
   $_SESSION['login'] = true;
   $_SESSION['ID'] = $row['PlayerID'];
   $_SESSION['username'] = $row['Username'];
   $_SESSION['email'] = $row['Email'];

   // Redirect
   header("Location: loggedin.php");

  }else{

   $error = true;
  }
 }else{

  $error = true;
 }
break;
}
if($error){

echo '<font color=red>Authentication failed</font>';
}
echo 'Display login form here';
?>

[cpd]

@wotw - Why have you used a switch statement with a single case? An if statement is a better control flow statement to use and you've actually done that hen setting the $case variable.

 

I wouldn't real_escape the password either. Just hash it using an appropriate method (suggest PHPass) and query the database with it.

[devilsvein]

Thanks for the feedback. I took the password escape out. But I want to know why I can login with a incorrect password on my site.

[Christian F.]

I suspect it has something to do with the escaping you've done, which potentially have altered the username and/or password. Also, without knowing how your registration code looks like, we're pretty much just guessing.

 

In any case, I would recommend you to read the following two articles:

http://michaelwright.me/php-password-storage

http://www.openwall.com/articles/PHP-Users-Passwords

 

(You too, wotw.)

[Pikachu2000]

Since this --> if ($hash == $dbpass) is the condition in the code above that causes $_SESSION['login'] to be set to TRUE, have you echoed $hash and $dbpass and compared them? If they match when a wrong password is entered, then you need to figure out why. If they don't match, and $_SESSION['login'] is still set to TRUE, you need to figure out why that's happening. That would be where I'd start anyhow.

[wotw]

I know all this. I used a switch because I normnally use a switch to do a password forgotten case and register.

 

Here is a quick secure class I wrote which you can use to secure your password:

 

<?php
class secure{

## GET A RANDOM SALT
function secure_random_salt(){
$randtext = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890';

$varlen = rand(5, 20);
$randtextlen = strlen($randtext);
$salt = '';

for($i = 0; $i < $varlen; $i++){

$salt .= substr($randtext,rand(1,$randtextlen),1);
}

return $salt;
}


## ENCODES PASSWORD
function secure_encode_password($password, $salt = ''){
if($salt == ''){

$salt = $this->secure_random_salt();
}

 return md5($password.$salt).':'.$salt;
}


## CHECK PASSWORDS MATCHES
function secure_check_password($password, $db_password){

$explode = explode(':', $db_password);
if(isset($explode[1])){

if($this->secure_encode_password($password, $explode[1]) == $db_password){

return true;
}else{

return false;
}
}else{

return false;
}
}
}
$secure = new secure;
?>

 

Simply including the class file and do this to create your password string.

 

// There is more to this class but I have cut it down. You could create a new function which will secure the posted values like the guys mention above.

$insert_password = $secure->secure_encode_password($password); // Password to insert into the db.

// And to check if the password is the same when they post it:


// $db_password is the actual password from the database.
// $password is the password posted from the login form.

if($secure->secure_check_password($password, $db_password)){
// Log the user in. $_SESSION etc..
}

Edited December 31, 2012 by wotw

[Christian F.]

return md5($password.$salt).':'.$salt;

Unfortunately, that's not secure. For more information, I recommend watching

.

[devilsvein]

I found the issue. It was a code which wasn't shown.

 

i had a snipet at the top of the page which was poorly designed.

 

It was suppose to redirect if the user was already logged in...but ws just logging in for the fun of it....

 

Thanks for your time and help