0

I have a change password script which is supposed to reset a users password with the values they provide assuming they match however this script is breaking something as users are not able to login with the password they provide to the script.

I'm not sure what is wrong as I also have an add_user script which is what I use to create the user accounts. The code to generate the password (hash) is the same and the data is successfully being put into the DB so I really don't know what is causing the problem. I'm guessing it has something to do with the data being provided prior to PHP hashing it thus in the DB it looks like it all went well as it's already hashed but I'm guessing if I was storing in plaintext it wouldn't be exactly the same as what the user entered otherwise the script would be working...

I've been working on the site all day so I'm really struggling to spot the error here.

I think my script used to work as this is the first I'm noticing the issue however I don't remember making any changes to this script in particular so cannot figure out why it would suddenly stop working.

session_start();
define('MyConst', TRUE);
include "includes/server.php";

if (!(isset($_SESSION['name']) && $_SESSION['name'] != ''))
{
    header("location:login.php");
}


$con = mysqli_connect($DB_HOST, $DB_USER, $DB_PASS, $DB_NAME) or die(mysqli_error($con));

$password1 = mysqli_real_escape_string($con, $_POST['newPassword']);
$password2 = mysqli_real_escape_string($con, $_POST['confirmPassword']);
$username = mysqli_real_escape_string($con, $_SESSION['name']);
$passwordhashed = password_hash("$password1", PASSWORD_DEFAULT);


if ($password1 <> $password2)
{
    echo "your passwords do not match";
    $referrer = $_SERVER['HTTP_REFERER'];
    header ("Refresh: 2;URL='$referrer'");
}

else if (mysqli_query($con, "UPDATE accounts SET password='$passwordhashed' WHERE username='$username'"))
{
    echo "You have successfully changed your password.";
    $referrer = $_SERVER['HTTP_REFERER'];
    header ("Refresh: 2;URL='$referrer'");
}
else
{
    mysqli_error($con);
}
mysqli_close($con);

Expected to check that passwords match and echo "You have successfully changed your password" if the change was successful and then redirect.

James Z
  • 12,209
  • 10
  • 24
  • 44
JBower
  • 77
  • 6
  • 2
    Which part of your if statement do you enter? You should at least be able to determine that much. – John Conde Jan 25 '19 at 03:00
  • How about adding an `die()` right after `echo "You have successfully changed your password.";`? Then you can see the flow of the code – Van Tho Jan 25 '19 at 03:04
  • @JohnConde what are you asking me? Enter what? – JBower Jan 25 '19 at 03:07
  • Include a screencast of your debugger usage for this code (if not a sufficient amount of input/variable states). PD of https://stackoverflow.com/questions/888/how-do-you-debug-php-scripts – mario Jan 25 '19 at 03:07
  • 2
    **WARNING**: When using `mysqli` you should be using [parameterized queries](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) to add user data to your query. **DO NOT** use manual escaping and string interpolation or concatenation to accomplish this because you will create severe [SQL injection bugs](http://bobby-tables.com/). Accidentally unescaped data is a serious risk. Using bound parameters is less verbose and easier to review to check you’re doing it properly. – tadman Jan 25 '19 at 03:08
  • 1
    **WARNING**: Writing your own access control layer is not easy and there are many opportunities to get it severely wrong. Please, do not write your own authentication system when any modern [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) like [Laravel](http://laravel.com/) comes with a robust [authentication system](https://laravel.com/docs/master/authentication) built-in. – tadman Jan 25 '19 at 03:08
  • @VanTho Thanks for your suggestion. I'll add this in the morning! Going to call it quits for tonight. Will it really spit out anything useful though? The script is adding to the database fine it's just not providing the correct password hash or something along those lines – JBower Jan 25 '19 at 03:09
  • 1
    When using `password_hash` it's important to put in the raw input, not something that's processed or escaped. Escaping is done *only* for immediate insertion, not prior to hashing. – tadman Jan 25 '19 at 03:09
  • 1
    Note: The object-oriented interface to `mysqli` is significantly less verbose, making code easier to read and audit, and is not easily confused with the obsolete `mysql_query` interface. Before you get too invested in the procedural style it’s worth switching over. Example: `$db = new mysqli(…)` and `$db->prepare("…")` The procedural interface is an artifact from the PHP 4 era when `mysqli` API was introduced and ideally should not be used in new code. – tadman Jan 25 '19 at 03:09
  • 1
    Note: A lot of problems can be detected and resolved by [enabling exceptions in `mysqli`](https://stackoverflow.com/questions/14578243/turning-query-errors-to-exceptions-in-mysqli) so any mistakes made aren’t easily ignored. Exceptions must be caught. Return values can be ignored, even important ones you must pay attention to. – tadman Jan 25 '19 at 03:10
  • 1
    What is the MySQL column data_type that you are using to store your password? Is it varchar(255), ? – Stephanie Temple Jan 25 '19 at 05:32

1 Answers1

0

The form page was missing a method as I had forgotten to add one in so it was defaulting to _GET when the rest of the script was referencing _POST.

Fix was simply adding method as post.

JBower
  • 77
  • 6