-1

I have built a website and I am now trying to create a way that allows users to reset their password whenever they forget. I have successfully created a password reset url link but anytime I click it I get an "Invalid request!" notification rather taking me to the index.php page. I would be grateful if anyone can help. My codes are below.

resetpassword.php

  include ("connect.php");

  //Connect to MySQL database using PDO.
 $pdo = new PDO("mysql:host=$host;dbname=$dbname", $user, $pwd);

 //Get the name that is being searched for.
 $email = isset($_POST['email']) ? trim($_POST['email']) : '';

 //The simple SQL query that we will be running.
 $sql = "SELECT `id`, `email` FROM `registration` WHERE `email` = :email";

 //Prepare our SELECT statement.
 $statement = $pdo->prepare($sql);

 //Bind the $name variable to our :name parameter.
 $statement->bindValue(':email', $email);

 //Execute the SQL statement.
 $statement->execute();

//Fetch our result as an associative array.
$userInfo = $statement->fetch(PDO::FETCH_ASSOC);

//If $userInfo is empty, it means that the submitted email
//address has not been found in our users table.
if(empty($userInfo)){
echo 'That email address was not found in our system!';
exit;
}

//The user's email address and id.
$userEmail = $userInfo['email'];
$userId = $userInfo['id'];

//Create a secure token for this forgot password request.
$token = openssl_random_pseudo_bytes(16);
$token = bin2hex($token);

//Insert the request information
//into our password_reset_request table.

//The SQL statement.
$insertSql = "INSERT INTO password_reset_request
          (user_id, date_requested, token)
          VALUES
          (:user_id, :date_requested, :token)";

//Prepare our INSERT SQL statement.
$statement = $pdo->prepare($insertSql);

//Execute the statement and insert the data.
$statement->execute(array(
"user_id" => $userId,
"date_requested" => date("Y-m-d H:i:s"),
"token" => $token
));

//Get the ID of the row we just inserted.
$passwordRequestId = $pdo->lastInsertId();

 //Create a link to the URL that will verify the
 //forgot password request and allow the user to change their
 //password.
 $verifyScript = 'http://localhost/trial/pages/createpassword.php';

//The link that we will send the user via email.
$linkToSend = "<a href='$verifyScript'? 
uid='.$userId.'&id='.$passwordRequestId.'&t='.$token'>$verifyScript.'? 
uid='.$userId.'&id='.$passwordRequestId.'&t='.$token</a>";

//Print out the email for the sake of this tutorial.
echo $linkToSend;

?>

createpassword

<form id="resetpasswordForm" action="verifypassword.php" class="loading-form" 
 method="POST">
<div class="form-group">
  <label for="email-input">Email</label>
  <input required name="email" type="email" class="form-control" id="email" 
title="An email is required">
</div>

<div class="form-group">
  <label for="password-input">Password</label>
  <input required type="password" name="password" class="form-control" 
id="pwd">
</div>

<div class="form-group">
  <label for="password-input">Confirm Password</label>
  <input required type="password" name="confirmpassword" class="form-control" 
 id="conpwd">
 </div>

<div class="form-group">
<!-- Do NOT use name="submit" or id="submit" for the Submit button -->
<button type="submit" name="ResetPasswordForm" class="btn btn-success">Reset 
Password</button>
</div>
<input type="hidden" name="uid" value="<?php echo $_GET['uid'];?>" />
<input type="hidden" name="t" value="<?php echo $_GET['t'];?>" />
<input type="hidden" name="id" value="<?php echo $_GET['id'];?>" />
</form>

verifypassword

include ("connect.php");

//Connect to MySQL database using PDO.
$pdo = new PDO("mysql:host=$host;dbname=$dbname", $user, $pwd);

//The user's id, which should be present in the GET variable "uid"
$userId = isset($_GET['uid']) ? trim($_GET['uid']) : '';
//The token for the request, which should be present in the GET variable "t"
$token = isset($_GET['t']) ? trim($_GET['t']) : '';
//The id for the request, which should be present in the GET variable "id"
$passwordRequestId = isset($_GET['id']) ? trim($_GET['id']) : '';

//Now, we need to query our password_reset_request table and
//make sure that the GET variables we received belong to
//a valid forgot password request.

$sql = "
  SELECT id, user_id, date_requested 
  FROM password_reset_request
  WHERE 
    user_id = :user_id AND 
    token = :token AND 
    id = :id
 ";

//Prepare our statement.
$statement = $pdo->prepare($sql);

 //Execute the statement using the variables we received.
 $statement->execute(array(
"user_id" => $userId,
"id" => $passwordRequestId,
"token" => $token
 ));

 //Fetch our result as an associative array.
 $requestInfo = $statement->fetch(PDO::FETCH_ASSOC);

 //If $requestInfo is empty, it means that this
 //is not a valid forgot password request. i.e. Somebody could be
 //changing GET values and trying to hack our
 //forgot password system.
 if(empty($requestInfo)){
 echo 'Invalid request!';
 exit;
 }

 //The request is valid, so give them a session variable
 //that gives them access to the reset password form.
 $_SESSION['user_id_reset_pass'] = $userId;

//Redirect them to your reset password form.
header('Location: index.php');
exit;

?>
Dee
  • 19
  • 1
  • 9
  • 1
    Please dont __roll your own__ password hashing .PHP provides [`password_hash()`](http://php.net/manual/en/function.password-hash.php) and [`password_verify()`](http://php.net/manual/en/function.password-verify.php) please use them. And here are some [good ideas about passwords](https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet) If you are using a PHP version prior to 5.5 [there is a compatibility pack available here](https://github.com/ircmaxell/password_compat) – RiggsFolly Aug 22 '18 at 14:58

2 Answers2

0

Modify this code :

$linkToSend = "<a href='$verifyScript'? 
uid='.$userId.'&id='.$passwordRequestId.'&t='.$token'>$verifyScript.'? 
uid='.$userId.'&id='.$passwordRequestId.'&t='.$token</a>";

To :

$linkToSend = '<a href="'.$verifyScript.'?uid='.$userId.'&id='.$passwordRequestId.'&t='.$token.'">'.$verifyScript.'?uid='.$userId.'&id='.$passwordRequestId.'&t='.$token.'</a>';

Or simply remove the accidental dots from the string

$linkToSend = "<a href='$verifyScript'? 
uid='$userId'&id='$passwordRequestId'&t='$token'>$verifyScript'? 
uid='$userId'&id='$passwordRequestId'&t='$token</a>";
RiggsFolly
  • 93,638
  • 21
  • 103
  • 149
Inazo
  • 488
  • 4
  • 15
  • You do know that `$vars` are expanded automatically in a double quoted string literal dont you? Which is why the OP was using single quotes (quite legal) around all the html attributes – RiggsFolly Aug 22 '18 at 14:24
  • And I am afraid your suggested change breaks the HTML! – RiggsFolly Aug 22 '18 at 14:24
  • Yes $vars with double quote are interpreted. But with the construction of is string it's looks like anormal the code will return this king of value uid='.12.' that's explain why he's got the error invalid request. If you take is code the html wasn't valid. And to finish do not use double quote to print variable content in string, for security reason and performance. – Inazo Aug 22 '18 at 14:32
  • Yes, so make that your suggested correction. Not one that makes things worse – RiggsFolly Aug 22 '18 at 14:34
  • i'm sorry but the solution i'll propose is correct. It will generate string like this for example : http://sqdsd.sqd/test.php?uid=12&id=azezaeza&t=213213azezae – Inazo Aug 22 '18 at 14:45
  • That would be a whole lot simpler and MUCH easier to read and maintain – RiggsFolly Aug 22 '18 at 14:48
  • Hum ok for you maybe but for me it's worse to read for me :) But thats developper style after . – Inazo Aug 22 '18 at 14:51
  • I spend quite a bit of time fixing other people code. Believe me there is nothing more likely to cause problems than unnecessary start/stop ... start/stops in string concatenation – RiggsFolly Aug 22 '18 at 14:55
  • In this case your right it's a larger point of error to start/stop string concatenation. – Inazo Aug 22 '18 at 14:57
  • I tried the above solution but it is still not working. – Dee Aug 22 '18 at 15:25
  • I don't understand your question. Please rephrase – Dee Aug 22 '18 at 15:38
-1

From what i could understand your variable is not passing any information so it might be due to your sql statement:

$requestInfo = $statement->fetch(PDO::FETCH_ASSOC);

I never used PDO so can't be 100% sure but try this out :

$id = htmlentities($connect->real_escape_string($_GET['id']));
$userid = htmlentities($connect->real_escape_string($_GET['uid']));
$token = htmlentities($connect->real_escape_string($_GET['token']));

$sql = " SELECT * FROM password_reset_request WHERE user_id = '$userid' AND token = '$token' AND id = '$id' ";
Gongas
  • 87
  • 7
  • Not really a good idea to take ___Safe___ a query and make it wide open to [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) in either the `MYSQLI_` or `PDO` API's – RiggsFolly Aug 22 '18 at 14:51
  • I guess that is not the point here? I commented to show him a way to solve the problem in the bad query he created i was not teaching him security standards ... – Gongas Aug 22 '18 at 15:29
  • Gongas I also think the problem is as you mentioned it because the code seems to fail at the point where it notifies you of an invalid request. However your solution is not working for me. – Dee Aug 22 '18 at 15:31
  • RiggsFolly I don't really understand. How is my code subject to sql injection attack? Can you please point out where the vulnerability is? – Dee Aug 22 '18 at 15:35
  • @Gongas you're not `preparing` your SQL query so your code is vulnerable to attacks such as SQL injections – Isaac Aug 22 '18 at 15:39
  • Read the links in the previous post, but basically anywhere you do `user_id = '$userid'` using input from the user, which could contain absolutely anything – RiggsFolly Aug 22 '18 at 15:40
  • @Isaac Did you mean to address that to Gongas – RiggsFolly Aug 22 '18 at 15:42
  • I already chhanged my answer , i know it was not sanitized but the point here was not to sanitize inputs ? i guess he can do it later . You could identify a security vulnerability in the code which was obvious but you could not provide an answer for the poster, i guess is just irrelevant to comment things like that, just my point of view – Gongas Aug 22 '18 at 16:00
  • @Dee Try changing your variables similar to what i did to this one to $userId = if(isset($_GET['uid']){ trim($_GET['uid']) } ; – Gongas Aug 22 '18 at 16:05
  • @Gongas I tried but I keep getting the unexpected 'if' (T_IF) syntax error. What am I not doing right? – Dee Aug 23 '18 at 09:34
  • My code was wrong i forgot to add an extra " ) " $userId = if(isset($_GET['uid'])){ trim($_GET['uid']); } – Gongas Aug 23 '18 at 22:46
  • If this does not work, debug the code, try to add an echo of the userid variable to see if it is getting it – Gongas Aug 23 '18 at 22:46