"Change password" help

[ViperBlade]

Hey. I'm trying to make a "change password" option for registered users of my website, but my code doesn't seem to work. I have two pages, pass.php
[code]<form method="POST" action="passprocess.php">
<p><table border="0" width="100%">
<tr>
<td>Current password:</td>
<td><input type="password" name="oldpass" size="25"></td>
</tr>
<tr>
<td>New password:</td>
<td><input type="password" name="newpass" size="25"></td>
</tr>
<tr>
<td>Confirm new password:</td>
<td><input type="password" name="confpass" size="25"></td>
</tr>
<tr>
<td>&nbsp;</td>
<td><input name="Submit" value="Submit" type="submit"></td>
</tr>
</form>
</p>
[/code]
and passprocess.php
[code]<?
session_start();

// Connect to DB
mysql_connect("localhost", "username", "password") or die(mysql_error());
mysql_select_db("example_db") or die(mysql_error());

// Assign variables
$uid = $_SESSION['userid']-1;
$oldpass = $_POST['oldpass'];
$newpass = $_POST['newpass'];
$confpass = $_POST['confpass'];
$num = mysql_query("SELECT password FROM users");
$dbpass = mysql_result($num, $uid);

// Check all fields were filled
if((!$newpass) || (!$confpass)){
    echo "One or more fields were left empty. Please try again.";
    exit();
}

// Check oldpass matches dbpass
if ( $oldpass <> $dbpass ){
echo $dbpass."<BR>".$_SESSION['userid']."<BR>".$oldpass;
echo "<BR>The old password you specified does not match the one in our database. Please try again";
exit();
}
else
{
if(mysql_query("UPDATE users SET password='$newpass' WHERE userid='$uid'")){
echo "<font face='Verdana' size='2' ><center>Thanks <br> Your password changed successfully. Please keep changing your password for better security</font></center>";
}
}
?>[/code]

Apparently everything works okay, because once the PHP is parsed it displays "Your password changed successfully...", but when I check my DB, there are no alterations.

Any help (and sorry for the long post),
ViperBlade

[Destruction]

Apart from a number of things that could be changed to optimise your code (such as only drawing the required user from the database and not the whole users list, which on a site with thousands of members would be very inefficient), you may want to look at:

[code]
<?php
$uid = $_SESSION['userid']-1;
?>
[/code]

Why are you taking 1 away from the userid?  Surely that will be updating a different user to the one intended?  Although I cannot know for sure as I can't see where that is being set.  You may want to check you're not updating the wrong user, and clear up certain areas like "SELECT `password` FROM `users` WHERE `userid` = $uid".  Also, make sure you're validating all the user input as at present you're open to SQL Injection by not doing so.

Hope this helps,

Dest

[ViperBlade]

Hey, thanks for ur help.
The messy code is on my to do list, don't worry :P and I'm relatively new to PHP/MySQL so I don't really know how to prevent sql injection.
The $uid-1 is a little odd, i'll agree. For some reason, if I don't subtract 1 it selects the wrong user.
Could you give me a few tips on preventing SQL injection and why it's saying "Password changed" when it's actually doing nothing at all.

Thanks,
Vipe