-1

I am writing this script for a time clock and one of the if statements are not working correctly. I have included the file below. I have changed some parts for privacy. It is when it checks to see if the username and current password are correct. The process executes correctly, but it is displaying incorrectly. It always says "Sorry, the username/password combination was not found. You must re-login now. It should say that the password was changed. This is the part of the code that is wierd. Any help is appreciated.

$sql = "SELECT * FROM tc_users WHERE userid = '$user_value' and password = '$current_pass'";
$result = mysql_query($sql);
if(mysql_num_rows($result) == 0) {
    echo "Sorry, that username/password combination was not found. You must re-login now.";
} else { 
if ($other_new_pass_value != $other_new_pass_confirm_value) {
        echo "Those passwords do not match, please go back and try again.";
    }else {
        echo "Password has successfully been changed. You must now re-login.";
    }
}

Also, I am trying to put that button on the bottom of the page. Is there a way to nest html into a php file without closing the php tags?

<head>
<title>Login Script</title>
<body bgcolor="#9966FF">
<link rel="icon" type="image/ico" href="favicon path"/>
</head>

<?php

define('DB_NAME', 'name');
define('DB_USER', 'user');
define('DB_PASSWORD', 'pass');
define('DB_HOST', 'host');


$link = mysql_connect(DB_HOST, DB_USER, DB_PASSWORD);

if (!$link){
    die('Could not connect: ' .mysql_error());
}

$db_selected = mysql_select_db(DB_NAME, $link);

if (!$db_selected) {
    die('Can\'t use ' . DB_NAME . ': ' . mysql_error());
}else

$user_value = $_POST['youruserid'];
$current_pass = $_POST['currentpass'];
$new_pass = $_POST['newpass'];
$new_pass_c = $_POST['confirmnewpass'];
$updatesql= "UPDATE tc_users SET password='$new_pass' WHERE userid = '$user_value'";
$updatequery = mysql_query($updatesql);
$sql = "SELECT * FROM tc_users WHERE userid = '$user_value' and password = '$current_pass'";
$result = mysql_query($sql);
if(mysql_num_rows($result) == 0) {
    echo "Sorry, that username/password combination was not found. You must re-login now.";
} else { 
    if ($other_new_pass_value != $other_new_pass_confirm_value) {
        echo "Those passwords do not match, please go back and try again.";
    }else {
        echo "Password has successfully been changed. You must now re-login.";
    }
}
mysql_close();
?>
<form action="login path" method="post" />
<input type="submit" value="Okay." />
</form>

Here is the form that the user submits.

<head>
    <title>Change User Password</title>
    <body bgcolor="#9966FF">
    <link rel="icon" type="image/ico" href="ico path"/>
</head>

<h3>This is the page to change your password.</h3>
<br>
<form action="chgpassprocess.php" method="post" />
<table>
    <tr>
        <td align="right">Your User ID: </td>
        <td align="left"><input type="text" name="youruserid"/></td>
    </tr>
    <tr>
        <td align="right">Current Password: </td>
        <td align="left"><input type="password" name="currentpass"/></td>
    </tr>
    <tr>
        <td align="right">New Password: </td>
        <td align="left"><input type="password" name="newpass"/></td>
    </tr>
    <tr> 
        <td align="right">Confirm New Password: </td>
        <td align="left"><input type="password" name="confirmnewpass"/></td>
    </tr>
    <tr>
        <td align="right"><input type="submit" value="Submit" /></td>
        <td align="left"><input type="reset" value="Reset Form" /></td>
    </tr>

</table>
</form>
<form method="GET" action="path">
<input type="submit" value="Cancel">
</form>
Adam Miller
  • 23
  • 1
  • 1
  • 11
  • `WHERE userid = '$user_value'` is that the username are you sure? – meda Jul 21 '14 at 21:32
  • yes. I am sure. The process is completed successfully. The password is changed properly. – Adam Miller Jul 21 '14 at 21:33
  • Is that your actual/entire form? `
    `
    – Funk Forty Niner Jul 21 '14 at 21:33
  • the form is on another page. what you are seeing is the button to re-login. I will edit the op to include the form. – Adam Miller Jul 21 '14 at 21:36
  • Your code is vulnerable to SQL injections. You should read on [how to prevent them in PHP](http://stackoverflow.com/q/60174/53114). – Gumbo Jul 21 '14 at 21:36
  • What about these variables? `$other_new_pass_value` and `$other_new_pass_confirm_value` (far as I know, they're stray variables) - We can't guess as to what should be what. Add error reporting to the top of your file(s) `error_reporting(E_ALL); ini_set('display_errors', 1);` see if it yields anything. – Funk Forty Niner Jul 21 '14 at 21:36
  • @AdamMiller you modified this code too much, I know you want to be safe but you have mistakes there – meda Jul 21 '14 at 21:38
  • You probably meant to use `$new_pass` and `$new_pass_c` instead of `$other_new_pass_value` and `$other_new_pass_confirm_value` - Plus, as per your edit, you're using GET `
    ` where it should be POST.
    – Funk Forty Niner Jul 21 '14 at 21:38
  • [**Please, don't use `mysql_*` functions in new code**](http://bit.ly/phpmsql). They are no longer maintained [and are officially deprecated](http://j.mp/XqV7Lp). See the [**red box**](http://j.mp/Te9zIL)? Learn about [*prepared statements*](http://j.mp/T9hLWi) instead, and use [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli) - [this article](http://j.mp/QEx8IB) will help you decide which. If you choose PDO, [here is a good tutorial](http://j.mp/PoWehJ). – esqew Jul 21 '14 at 21:39
  • $other_new_pass_confirm_value: that is the reason that it is not working. Thank you. I did not realize that I was using the wrong variables. I am aware that I am opening up sql injection holes, but it is a local system, so I am not worried about it. – Adam Miller Jul 21 '14 at 21:41
  • Problem solved I take it? – Funk Forty Niner Jul 21 '14 at 21:42
  • @Fred-ii- That button still works just fine with get. – Adam Miller Jul 21 '14 at 21:42
  • *"$other_new_pass_confirm_value: that is the reason that it is not working. Thank you. I did not realize that I was using the wrong variables"* - You're welcome. – Funk Forty Niner Jul 21 '14 at 21:44
  • Actually, I was wrong. That was an error, but it was not the error. The problem was still there. The answer below worked. But thank you anyways @Fred-ii- ! – Adam Miller Jul 21 '14 at 21:46

1 Answers1

0

The logic flow is incorrect because you update:

UPDATE tc_users SET password='$new_pass' 
WHERE userid = '$user_value'

then check with the old password

SELECT * 
FROM tc_users 
WHERE userid = '$user_value' and password = '$current_pass'

You want to do the opposite, check first then update:

if(mysql_num_rows($result) == 0) {
    echo "Sorry, that username/password combination was not found....";
} else { 
    if ($other_new_pass_value != $other_new_pass_confirm_value) {
        echo "Those passwords do not match, please go back and try again.";
    }else {
        //move the update code here to only run when the old one is found
        //and the new one matches its confirmation
        echo "Password has successfully been changed. You must now re-login.";
    }
}
meda
  • 45,103
  • 14
  • 92
  • 122