Dh1472 Posted August 29, 2013 Share Posted August 29, 2013 I am new at coding, and at PHP. I am attempting to update a password. I have inserted my code, including comments and mark throughs. I am at the point where the database updates, however, it does not product a new $salt variable, and it does not encrypt using sha1 again, either. It is just putting $salt = 0 and $pwd = the literal new password. Also, the code has been changed SO many times to try to force the new encryption, it is probably very messy, I apologize ahead of time! Again, any help is appreciated! <?php require('includes/connection.inc.php'); $OK = false; $done = false; //db connection $conn = dbConnect('write', 'pdo'); if (isset($_GET['user_id']) && !$_POST) { //prepare query $sql = 'SELECT user_id, salt, pwd FROM lr1 WHERE user_id=?'; $stmt = $conn->prepare($sql); //bind the results $stmt->bindColumn(1, $user_id); $stmt->bindColumn(3, $salt); $stmt->bindColumn(4, $password); //execute $OK = $stmt->execute(array($_GET['user_id'])); $stmt->fetch(); } //if form is submitted, update record if (isset($_POST['update'])) { $password = trim($_POST['pwd']); $retyped = trim($_POST['conf_pwd']); require_once('classes/CheckPassword.php'); $errors = array(); //min password length set below $checkPwd = new Ps2_CheckPassword($password, 10); $checkPwd->requireMixedCase(); $checkPwd->requireNumbers(2); $checkPwd->requireSymbols(); $passwordOK = $checkPwd->check(); if (!$passwordOK) { $errors = array_merge($errors, $checkPwd->getErrors()); } if ($password != $retyped) { $errors[] = "Your passwords don't match."; } if (!$errors) { // encrypt the password and salt with SHA1 //include the connection file $conn=dbConnect('write', 'pdo'); //create a salt using the current timestamp //encrypt pwd and salt $salt = time(); $pwd = sha1($password . $salt); //original had salt=?, pwd=? $sql ='UPDATE lr1 SET salt=?,pwd=? WHERE user_id=?'; $stmt = $conn->prepare($sql); //$stmt->bindColumn(3, $salt); //$stmt->bindColumn(4, $pwd); //$stmt->bindParam(':user_id', $user_id); $done = $stmt->execute(array($_POST['salt'], $_POST['pwd'], $_POST['user_id'])); // execute query by passing array of variables if($stmt->rowCount() == 1) { $success = "The username has been updated. You may now log in."; } else { $errors[] = 'Sorry, there was a problem with the database.'; } } } ?> <!doctype html> <html> <head> <meta charset="utf-8"> <title>Update Password</title> <link href="portal.css" rel="stylesheet" type="text/css"> </head> <body> <img src="images/dcanew.jpg" alt="dca header image" class="imgheader" /> <div id="wrapper"> <br><br> <form id="form1" method="post" action=""> <?php if (isset($success)){ echo "<p>$success</p>"; } elseif (isset($errors) && !empty($errors)) { echo '<ul>'; foreach ($errors as $error) { echo "<li>$error</li>"; } echo '</ul>'; } ?> <input name="user_id" type="hidden" value="<?php echo $user_id; ?>"> <p> <label for="pwd">New Password: </label> <input type="password" name="pwd" id="pwd" /> </p> <p> <label for="conf_pwd">Re-enter New Password:</label> <input type="password" name="conf_pwd" id="conf_pwd"> </p> <p> <input name="update" type="submit" id="update" value="Update Password" /> </p> </form> {more on page, but not included here} Quote Link to comment Share on other sites More sharing options...
Muddy_Funster Posted August 29, 2013 Share Posted August 29, 2013 Please use code tags when posting code on the forum. A couple of questions - why are you using bindColumn? why are you using sha1? and why are you storing a salt in the database? Quote Link to comment Share on other sites More sharing options...
Dh1472 Posted August 29, 2013 Author Share Posted August 29, 2013 For the bindColumn - I was using bindParam - and received the same issue. As to why - I am unsure. The Salt is part of the login parameters, so the encoded salt would be compared with the password and encoding to login to the page. I am using sha1 because I have not worked up to md5 yet, although that would be more secure. Thank you for looking at it. Quote Link to comment Share on other sites More sharing options...
Muddy_Funster Posted August 29, 2013 Share Posted August 29, 2013 both sha1 and md5 are outdated. using a blowfish crypt with self generating salt, similar to the one in my signature, would be the way to go. you shouldn't store hash salts in the database except in very rare occasions, doing so unwittingly can open you up to packet attacks as you are forced to pull the hash and the salt back out of the database to perform the check. stick to using either bindParam or bindValue when attaching values to parameters for prepared statements. Quote Link to comment Share on other sites More sharing options...
Dh1472 Posted August 29, 2013 Author Share Posted August 29, 2013 Thank you! Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted August 29, 2013 Share Posted August 29, 2013 i see an amount of code/data that isn't doing anything and isn't needed. i recommend you start by defining what the posted code is supposed to do - allow the currently logged in user to update his password. you would typically require the old password to be entered (in case a session has been hijacked), along with the new one, plus the new one retyped. all the posted code (form processing and the form) should be inside a conditional statement that has checked if the current visitor is logged in (in which case you would know the user_id from the login/authentication code, and not need to pass it as a get variable or as a hidden form field, which is insecure for this operation anyway since that would let anyone try to update/screw-up someone else's password.) bindColumn is for binding the columns you have SELECT'ed and it's generally not needed. you can just fetch the resultant row(s).also, the numerical parameter is the column number in the SELECT term, it's not the actual column numbers in your table. you are suppling the input data in your $stmt->execute() statement. there's no need to bind the inputs in this case, though you get more control/checking if you do bind the inputs. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.