Help with crypt(), UPDATE and Comparisons

[Hexagon]

Hi guys,

 

i am creating my change password site for my website and i have some problems with the code...

 

For some reason i have difficulties with the passwords being compared and replaced in the db after crypting them.

 

I wanted this:

 

Either get the current users password and compare it to the input value of $oldpass or compare the input value of $oldpass with the password stored in the database for the current user.

 

After checking if the $oldpass and the password from the database match and IF they match then take the input value of $newpass and $repeatpass, compare them and if they match, then crypt() $newpass and update the database with the new password.

 

 I am not even sure if the passwords are even crypted.

 

Also in the code i am comparing $oldpass with $_SESSION['password'] which is not the password from the db, i can't figure out how to call the password from the db.

 

Thanks in advance!

<?php

include 'check_login_status.php';

$u="";
$oldpass=md5($_POST['oldpass']);
//stripping both strings of white spaces
$newpass = preg_replace('#[^a-z0-9]#i', '', $_POST['newpass']);
$repeatpass = preg_replace('#[^a-z0-9]#i', '', $_POST['repeatpass']);

//get the username from the header
if(isset($_GET["u"])){
	$u = preg_replace('#[^a-z0-9]#i', '', $_GET['u']);
} else {
    header("location: compare_pass.php?u=".$_SESSION["username"]);
    exit();	
}

// Select the member from the users table
$sql = "SELECT password FROM users WHERE username='$u' LIMIT 1";
mysqli_query($db_conx, $sql);
$user_query = mysqli_query($db_conx, $sql);
// Now make sure that user exists in the table
$numrows = mysqli_num_rows($user_query);
if($numrows < 1){
	echo "That user does not exist or is not yet activated, press back";
    exit();	
}

if ($oldpass == $_SESSION['password']) {
	echo "session and oldpass are matching";
} else {
	echo "Session and oldpass do not match!";
}

$isOwner = "no";
//check if user is logged in owner of account
if($u == $log_username && $user_ok == true){
	$isOwner = "yes";
}

$passhash = "";
if (isset($_POST["submit"]) && ($isOwner == "yes") && ($user_ok == true) && ($newpass == $repeatpass)) {
	$passhash = crypt_sha256("$newpass", "B-Pz=0%5mI~SAOcW0pMUdgKQh1_B7H6sbKAl+9~O98E9MBPrpGOtE65ro~8R");
	$sql = "UPDATE users SET `password`='$passhash' WHERE username='$u' LIMIT 1";
}

if (mysqli_query($db_conx, $sql)) {
    echo "Record updated successfully";
	
} else {
    echo "Error updating record: " . mysqli_error($db_conx);
	}

?>

<h3>Create new password</h3>
  <form action="" method="post">
    <div>Current Password</div>
	<input type="text" class="form-control" id="password" name="oldpass" >
	<div>New Password</div>
    <input type="text" class="form-control" id="password" name="newpass" >
	<div>Repeat Password</div>
	<input type="text" class="form-control" id="password" name="repeatpass" >
    <br /><br />
    <input type="submit" name="submit" value="Submit"> 
    <p id="status" ></p>
  </form><?php echo "{$oldpass}, {$_SESSION['password']}"; ?>
  
  
  <pre>
  <?php
  var_dump($_SESSION);
	var_dump($oldpass);
	var_dump($passhash);
	var_dump($newpass);
	var_dump($repeatpass);
	?>
  </pre>

Edited February 20, 2015 by Hexagon

[mac_gyver]

your program logic makes no sense. i recommend that you first define what inputs your code needs and will receive from your form and from your login system, then define what sort of processing you are trying to accomplish based on those inputs.

 

some suggestions -

 

1) ALL the form processing logic needs to be inside of a conditional statement that is only true when the form has been submitted. this is so that your code doesn't produce a bunch of php errors and error messages from your logic when there isn't any $_POST data.

 

2) to compare the oldpass value with the current password hash stored in the database table, you will need to use the same hashing logic on the value from the form that was used when the current password hash was produced and stored. you will also need to fetch the current password hash from the database table.

 

3) if you don't already have real data stored for user's, you should be using php's password hash functions - http://php.net/manual/en/book.password.php

 

4) all the posted code is dependent on the visitor being logged in (i assume that check_login_status.php does this.) therefore, you should not be using any $_GET variable to tell your code who the current visitor is. you should be getting the current visitor's id from your login system.

 

5) you are running the SELECT... query two times in a row. why?

[Hexagon]

First, thanks for your input, I have changed the code dramatically and reverted to md5 just so i can test everything because i have no experience with the password_hash.

<?php

$u="";

//stripping both strings of white spaces
$newpass = preg_replace('#[^a-z0-9]#i', '', $_POST['newpass']);
$repeatpass = preg_replace('#[^a-z0-9]#i', '', $_POST['repeatpass']);

if(isset($_GET["u"])){
	$u = preg_replace('#[^a-z0-9]#i', '', $_GET['u']);
} else {
    header("location: settings.php?u=".$_SESSION["username"]);
    exit();	
}

// Select the member from the users table
$sql = "SELECT * FROM users WHERE username='$u' AND activated='1' LIMIT 1";
$user_query = mysqli_query($db_conx, $sql);
// Now make sure that user exists in the table
$numrows = mysqli_num_rows($user_query);
if($numrows < 1){
	echo "That user does not exist or is not yet activated, press back";
	
    exit();	
}


$isOwner = "no";
//check if user is logged in owner of account
if($u == $log_username && $user_ok == true){
	$isOwner = "yes";
}


if (isset($_POST["submit"]) && ($isOwner == "yes") && ($user_ok == true) && ($newpass == $repeatpass)) {
	$newpass = md5($newpass);

	$sql = "UPDATE `users` SET `password`='$newpass' WHERE username='$u'";
if (mysqli_query($db_conx, $sql)) {
    echo "Record updated successfully with $newpass";
} else {
    echo "Error updating record: " . mysqli_error($conn);
}
}
?>


  
  
  <pre>
  <?php
  var_dump($_SESSION);


	var_dump($newpass);
	var_dump($repeatpass);
	?>
  </pre>

Thats the new code, I will try to implement a better encryption later.

But i am still trying to figure out how to replace the get['u'] logic.