0

I am creating a user form and form action where the user (already logged in using session variable) can change their md5 (i know MD5 is outdated and unsecured, this is for test purposes) encrypted account password stored in the sql database 'users' table.

I have a form which requests the inputs 'currentpassword', 'newpassword' and 'confirmnewpassword'. The form passes the entered data to passwordaction.php using $_POST.

The username is acquired from the $_SESSION 'autheticatedUser' and passwords acquired from the previous $_POST form variables. I then use an sql statement to get the password from the database for comparison to 'currentpassword' variable, DOES THIS COUNT AS INSECURE CLIENT SIDE VALIDATION? ?

I then have an SQL UPDATE statement to update the password row of the specified user in the database and the user is redirected and notified of success or failure using $_SESSION headers.

I have been reading and re-reading through my code trying to figure out where ive gone wrong as when trying to change a user account password I keep being returned to my login page (using $SESSION header) telling me it has updated properly however when i check the database the password has not been updated.

Im hoping someone elses view or perspective may be able to help me see what ive missed, can anyone suggest why my sql UPDATE statement is not working?

any constructive criticism welcome

below is my code for the 'action' php page

 <?php

session_start();

$username = $_SESSION["authenticatedUser"];
$currentpassword = md5($_POST['currentpassword']);
$newpassword = md5($_POST['newpassword']);
$confirmnewpassword = md5($POST['confirmnewpassword']);

/* make a connection with database */
$con = mysql_connect("localhost", "root", "") or die(mysql_error());

/* select the database */
mysql_select_db("groupproject") or die(mysql_error());

$queryget = mysql_query("SELECT password FROM users WHERE username='$username'") or 
die(mysql_error());
$row = mysql_fetch_assoc($queryget);
 $currentpasswordDB = $row['password'];

//check passwords

if ($currentpassword==$currentpasswordDB)

{
if ($newpassword==$confirmnewpassword)
{
//success, change password in DB
    $querychange = mysql_query("UPDATE users SET password='$newpassword' WHERE       
 username='$username'") or die(mysql_error());
}
else header("Location: passwordmismatch.php");

if ($querychange == true){

    $_SESSION["passchange"] = "Your password has been changed, Please Log in";

    header("Location:login.php");

}

else $_SESSION["nopasschange"] = "Your password could not be changed, Please try   
again";
 header("Location:userchangepassword.php");

}

else header("Location: passwordmismatch.php");

mysql_close($con);

?>
Adam Outram
  • 89
  • 3
  • 10
  • 2
    You are using [an **obsolete** database API](http://stackoverflow.com/q/12859942/19068) and should use a [modern replacement](http://php.net/manual/en/mysqlinfo.api.choosing.php). You are also **vulnerable to [SQL injection attacks](http://bobby-tables.com/)** that a modern API would make it easier to [defend](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) yourself from. – Quentin Dec 08 '12 at 19:39
  • 1
    MD5 is [not suitable for password hashing](http://php.net/manual/en/faq.passwords.php#faq.passwords.fasthash), see also the [password storage cheatsheet](https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet). – Quentin Dec 08 '12 at 19:40
  • both comments are helpful thank you, however this project will never be a live website. I will bear in mind that MD5 is not the optimal hashing facility – Adam Outram Dec 08 '12 at 20:12

2 Answers2

1

$user and $username are different variables.

Quentin
  • 914,110
  • 126
  • 1,211
  • 1,335
0

Bit late :P

but in the row $confirmnewpassword = md5($POST['confirmnewpassword']);

it should be

$confirmnewpassword = md5($_POST['confirmnewpassword']);

Michael
  • 1