Password recovery program

Question

This is a password recovery program I made, and I just want it checked out. These aren't all the files for the login and register system, only the password recovery part. The columns in the users table are id, username, password, salt, passres, and passexp. passres is the reset token, and passexp is the expiration time for the url. I know I am supposed to mail them the link and not just show it to them, but I don't have an email server, so that is what I am doing to test it.

resetpass.php:

<?php
 require_once('conn.php');
 if (isset($_POST['user']) && !empty($_POST['user'])) {
 $us = $_POST['user'];
 $query = $con->query("SELECT * FROM users WHERE username = '".$con->real_escape_string($us)."' LIMIT 1");
 if ($query->num_rows === 1) {
 $row = $query->fetch_assoc();
 do {
 $key = substr(str_shuffle("0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ`~!@#$%^&*()-_=+|[]{}:<>./?"), 0, 50);
 } while($con->query("SELECT passres FROM users WHERE passres = '".$con->real_escape_string($key)."'")->num_rows > 0);
 $hash = hash('sha256', $key);
 $date = time() + 172800;
 if ($con->query("UPDATE users SET passres = '".$con->real_escape_string($hash)."', passexp = '".$con->real_escape_string($date)."' WHERE id = ".$con->real_escape_string($row['id']))) {
 echo '<a href="reset.php?key='.urlencode($key).'&user='.urlencode($row['username']).'">http://localhost/login/reset.php?key='.$key.'&user='.urlencode($row['username']).'</a>';
 } else {
 echo ('An error occured.');
 }
 } else {
 header('Location:'); exit();
 }
 } else {
 ?>
 <form action='' method='post'>
 <input type="text" name="user" placeholder="Enter Username">
 <input type="submit" value="Submit">
 </form>
 <?php
 }
?>

reset.php:

<?php
 require_once('conn.php');
 if (isset($_GET['key'], $_GET['user']) && !empty($_GET['key']) && !empty($_GET['user'])) {
 $key = $_GET['key'];
 $us = $_GET['user'];
 $query = $con->query("SELECT * FROM users WHERE username = '".$con->real_escape_string($us)."' LIMIT 1");
 if ($query->num_rows === 1) {
 $row = $query->fetch_assoc();
 if (isset($row['passres'], $row['passexp'])) {
 if (hash('sha256', $key) == $row['passres']) {
 if ($row['passexp'] <= time()) {
 echo 'This link has expired!';
 $con->query("UPDATE users SET passres = NULL, passexp = NULL WHERE username = '".$con->real_escape_string($us)."'");
 } else {
 ?>
 <form action='reset.php' method='post'>
 <input style="display: none;" type='text' name='key' value='<?php echo $key ?>'>
 <input style="display: none;" type='text' name='user' value='<?php echo $us ?>'>
 <input type='password' name='pass' placeholder='New Password'> <?php echo (isset($_GET['er'])) ? $_GET['er'] : ''; ?><br>
 <input type='password' name='cpass' placeholder='Confirm Password'><br>
 <input type='submit' value='Update Password'>
 </form>
 <?php
 }
 } else {
 echo 'this page does not exist!';
 }
 } else {
 echo 'this page does not exist!';
 }
 } else {
 echo 'this page does not exist!';
 }
 } elseif (isset($_POST['key'], $_POST['user'], $_POST['pass'], $_POST['cpass']) && !empty($_POST['key']) && !empty($_POST['user']) && !empty($_POST['pass']) && !empty($_POST['cpass'])) {
 $key = $_POST['key'];
 $us = $_POST['user'];
 $pass = $_POST['pass'];
 $cpass = $_POST['cpass'];
 if ($pass !== $cpass) {header('Location: reset.php?key='.urlencode($key).'&user='.urlencode($us).'&er=The+passwords+don\'t+match!'); exit();}
 if (strlen($pass) < 8) {header('Location: reset.php?key='.urlencode($key).'&user='.urlencode($us).'&er=The+password+needs+to+be+at+least+8+characters'); exit();}
 $query = $con->query("SELECT * FROM users WHERE username = '".$con->real_escape_string($us)."' LIMIT 1");
 if ($query->num_rows === 1) {
 $row = $query->fetch_assoc();
 if (isset($row['passres'], $row['passexp'])) {
 if (hash('sha256', $key) == $row['passres']) {
 if ($row['passexp'] <= time()) {
 echo 'This link has expired!';
 $con->query("UPDATE users SET passres = NULL, passexp = NULL WHERE username = '".$con->real_escape_string($us)."'");
 } else {
 $pass = sha1(md5($pass).sha1($row['salt']));
 if ($con->query("UPDATE users SET password = '".$con->real_escape_string($pass)."', passres = NULL, passexp = NULL WHERE username = '".$con->real_escape_string($us)."'")) {
 echo 'Password Reset! click <a href="index.php">here</a> to sign in';
 }
 }
 } else {
 echo 'this page does not exist!';
 }
 } else {
 echo 'this page does not exist!';
 }
 } else {
 echo 'this page does not exist!';
 }
 } else {
 header('Location: resetpass.php'); exit();
 }
?>

conn.php:

<?php
 session_start();
 $con = new mysqli('localhost', 'root', '**********', 'test');
?>

Answer 1

Security practices and correctness

It's probably a good idea to keep an audit log of the times and client IP addresses for all attempted and completed password changes.

<?php echo $us ?> and <?php echo (isset($_GET['er'])) ? $_GET['er'] : ''; ?> are vulnerable to cross-site scripting (or HTML injection) attacks. You should take care to call htmlspecialchars(...) whenever emitting HTML.

Instead of <input style="display: none;" ...>, you should use <input type="hidden" ...>, which is more straightforward. In fact, some browsers have been known to not submit those display: none form fields at all, due to an implementation bug or misinterpretation of the CSS specification.

Salted MD5 and SHA hashes are no longer considered good password-storage mechanisms. If you have PHP ≥ 5.5.0, you should use password_hash(), which uses the bcrypt algorithm.

Implementation

There is also a lot of related code between resetpass.php and reset.php, such as the passres generator and verifier, that should be extracted into functions and placed in a file that is included by both pages.

The indentation in reset.php is very deep. Keeping in mind that this usually one way to succeed but many ways to fail, it would help to invert your conditions such that the error handler comes first.

if (query->num_rows < 1) {
 header('HTTP/1.0 404 Not Found');
 exit();
}
$row = $query->fetch_assoc();
if (!isset($row['passres'], $row['passexp']) ||
 hash('sha256', $key) != $row['passres']) {
 header('HTTP/1.0 404 Not Found');
 exit();
} elsif ($row['passexp'] <= time()) {
 echo 'This link has expired!';
 ...
 exit();
}
$pass = ...;
...

Answer 2

I found some issues. First of all is the two password does not match and password length error messages. Where are they displayed? I think they are just passed in URL.

The hardcoded URL with those error messages are ugly. Change the error messages to plain text and use urlencode. It reduces the risk of typos.

In the checking if you use numbers to check the expiry and password length. Use config or well named constants to make your code well readable and configurable.

Avoid the abbreviations in your variable because they are not well readable.

There is a link in your passreset.php where the link's text is http://localhost/... Use $_SERVER['HTTP_HOST'] to use the same script on different environment. Eg.: test, demo, production.

• gommb: the errors are displayed on this line <input type='password' name='pass' placeholder='New Password'> <?php echo (isset($_GET['er'])) ? $_GET['er'] : ''; ?><br>