[fixed]password inquires[fixed] :))

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Post by Mordred »

Listen, just stop trying things out without understanding them. In that last script you have all the errors that were pointed earlier:

1. Use $sPassword instead of $_POST['password'] EVERYWHERE IN YOUR QUERIES (but not in the comparison)
2. Again, you have "_$POST", this time in a code that's not executed
3. '$_POST[password]' is a nice constant string WHICH IS NOT THE SAME as the variable $_POST['password']

Btw
4. hash('md5', ...) is the same as md5(...) and the latter is backward compatible to much older versions of PHP
5. {$_SESSION['logname']} and {$_POST['new_pass']} shouldn't be in a query as well, ecape them like the password.

6. Hashing without salt is not secure enough, here's a couple of articles on the subject:
http://phpsec.org/articles/2005/password-hashing.html
(shameless plug mode on) viewtopic.php?t=62782

Of course, make this code run correctly first before trying any modifications.

I strongly suggest you re-read the chapters about strings in the manual, and/or just don't put array lookups in double-quoted strings.
Turn your error reporting to the max (error_reporting(E_ALL);) and use a decent editor with syntax highlighting - you would have noticed all of these errors had you done so.

Good luck ;)
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

In your update query, you have _$POST instead of $_POST in your md5 hashing.
User avatar
Obadiah
Forum Regular
Posts: 580
Joined: Mon Jul 31, 2006 9:13 am
Location: Ashland, KY
Contact:

Post by Obadiah »

First of all mordred thanks for your help and speady reply but this statement
Mordred wrote: Listen, just stop trying things out without understanding them.
is hardly nessasary since a great deal of us here are new to the language and most of the time are thrown into projects due to work and must have them completed and i will be the first to admit...i dont know it all...but who does...what i did in order to correct the error on the first SQL statement was change the second condition on it....a condition that you suggested
Mordred wrote: In that last script you have all the errors that were pointed earlier
dude...did you even read through the last post correctly or for that matter have been following the thread...yesterday i was getting different password hashes whether they were hashed in MD5 or SHA256 today that problem is solved
Mordred wrote: 5. {$_SESSION['logname']} shouldn't be in a query as well, ecape them like the password.
if you look at the query i wrote when i used that...i was pointing to user_name saying that if that user is looged in look at his password...and from there change it
Mordred wrote: 1. Use $sPassword instead of $_POST['password'] EVERYWHERE IN YOUR QUERIES (but not in the comparison)
thanks will test this out :)
Mordred wrote: 2. Again, you have "_$POST", this time in a code that's not executed
where, how do i execute it?
Mordred wrote: 3. '$_POST[password]' is a nice constant string WHICH IS NOT THE SAME as the variable $_POST['password']
ok? :?
Mordred wrote: I strongly suggest you re-read the chapters about strings in the manual, and/or just don't put array lookups in double-quoted strings.
Turn your error reporting to the max (error_reporting(E_ALL);) and use a decent editor with syntax highlighting - you would have noticed all of these errors had you done so.
Good luck ;)
thanks for the well wishes....will do after i get this damn thing to work properly....now can someone properly answer the question that i asked please
User avatar
Obadiah
Forum Regular
Posts: 580
Joined: Mon Jul 31, 2006 9:13 am
Location: Ashland, KY
Contact:

Post by Obadiah »

thanks everah i tried switching it to this

Code: Select all

$sql = "UPDATE customer SET password '{$_POST['new_pass']}' WHERE password = hash('md5','$_POST[password]')";
and its still spitting me out the same stuff
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Post by Mordred »

Obadiah wrote:First of all mordred thanks for your help and speady reply but this statement
Mordred wrote: Listen, just stop trying things out without understanding them.
is hardly nessasary since a great deal of us here are new to the language ...
I didn't mean to offend, I meaned what I said - you must strive to understand things, otherwise thoughtless tweaking of this and that will get you nowhere, as demonstrated. It's not a sin not to know something, but it's a sin to try to avoid learning it.

If you were at least as concerned about getting results as to do a search for the things I've mentioned, you'd have found this:

Code: Select all

hash('md5','$_POST[password]')
which is wrong, and should be this:

Code: Select all

hash('md5',$_POST['password'])
And if you want your software to work, follow the other advices as well.
User avatar
Obadiah
Forum Regular
Posts: 580
Joined: Mon Jul 31, 2006 9:13 am
Location: Ashland, KY
Contact:

Post by Obadiah »

took your advice...and ive cleared my other errors but the update query now is giving me issues

Code: Select all

$sql = "UPDATE customer SET password md5('$_POST[new_password]') WHERE password = md5('$_POST[password]')";
its not hashing the passwords when it gives me the sql error either...shouldent it be?
User avatar
Obadiah
Forum Regular
Posts: 580
Joined: Mon Jul 31, 2006 9:13 am
Location: Ashland, KY
Contact:

Post by Obadiah »

WOOT!!! FINAL POST FTW (For The Win):D

Code: Select all

<?php 
session_start();
error_reporting(E_ALL); 
ini_set('display_errors', 1); 

function doDB() 
{ 
    $conn = mysql_connect("PLAY","WoW","JERBRONIES") or die(mysql_error()); 
    mysql_select_db("customerdirectory",$conn) or die(mysql_error()); 
    return $conn; 
} 

$conn = doDB(); 
$next_program = "changed.php";
$sPassword = mysql_real_escape_string($_POST['password']);
$sql = "Select user_name, password FROM customer WHERE user_name= '{$_SESSION['logname']}' AND password = md5('$_POST[password]')";
$result = mysql_query($sql,$conn) or die(mysql_error()); 
while ($newArray = mysql_fetch_array($result))
{
	$password = $newArray['password'];
	$user_name = $newArray['user_name'];
}
$nPassword = mysql_real_escape_string(hash('md5',$_POST['new_password']));
if (hash('md5',$_POST['password']) === $password) {
$sql = "UPDATE customer SET password = md5('$_POST[new_password]') WHERE password = md5('$_POST[password]')";
$result = mysql_query($sql,$conn) or die(mysql_error());
header("Location: $next_program");
}
else{
	echo"incorrect password matchup please try again";
}

?>
darn '= sign' screwed up the whole thing :P
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Post by Mordred »

Okay, you got it more or less working ;) Now get it fully working and secured:

1. Register two users with the same password, change one and try the other.

2. Try a user with a password O'Henry

3. Try a user with a username O'Mara

4. Depending on your database, there is a range of other nastynesses that could be done, but I'm too lazy to think of them offline.

Hint:
$sPassword = mysql_real_escape_string($_POST['password']);

You were supposed to use this variable in the queries, weren't you?

Btw the "s" before the password is the so called Hungarian notation, it means to me that $sPassword is a string.

Also, this line does nothing:
$nPassword = mysql_real_escape_string(hash('md5',$_POST['new_password']));

From where does "next_program" come here:
"Location: $next_program"
It could potentially be very dangerous, but don't think about it now, concentrate on the tasks above, you're almost there!
User avatar
Obadiah
Forum Regular
Posts: 580
Joined: Mon Jul 31, 2006 9:13 am
Location: Ashland, KY
Contact:

Post by Obadiah »

Mordred wrote: Hint:
$sPassword = mysql_real_escape_string($_POST['password']);

You were supposed to use this variable in the queries, weren't you?
dude i tried....though knowing me i prolly screwed it up but everytime i had it in there PHP started its complaining crap
PHP wrote: Whaa...error on line 36 you noob
Whaa....you didnt use the sql query right
Whaa....go Play WoW
Whaa Smurfslap
ok ok so it didnt say the last 2...but someday it might :lol:
Mordred wrote: Also, this line does nothing:
$nPassword = mysql_real_escape_string(hash('md5',$_POST['new_password']));
:oops: i actually forgot all about that line...i was testing something and forgot to take that out...nice catch

i am gonna try again on the $sPassword variable and run the test you sugested...thanks for the assist bro :)
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

I might try something like this...

Code: Select all

<?php
session_start();
error_reporting(E_ALL);
ini_set('display_errors', 1);

function doDB()
{
    $conn = mysql_connect("PLAY","WoW","JERBRONIES") or die(mysql_error());
    mysql_select_db("customerdirectory",$conn) or die(mysql_error());
    return $conn;
}

$conn = doDB();
$next_program = "changed.php";
$sPassword = md5($_POST['password']);
$sql = "Select `user_name`, `password` FROM `customer` WHERE `user_name`= '{$_SESSION['logname']}' AND password = '$sPassword'";
$result = mysql_query($sql,$conn) or die(mysql_error());

// Default $user_name
$user_name = null;
while ($newArray = mysql_fetch_array($result))
{
        $password = $newArray['password']; // This is unneeded since you have it from post
        $user_name = $newArray['user_name'];
}

$nPassword = hash('md5',$_POST['new_password']);

// This entire piece of logic can now be summed up into
if (!is_null($user_name)) {
  $sql = "UPDATE customer SET password = '$nPassword' WHERE password = '$sPassword'";
  $result = mysql_query($sql,$conn) or die(mysql_error());
  header("Location: $next_program");
  exit; // ALWAYS REMEMBER TO EXIT AND YOU SHOULD USE A COMPLETE URI IN HEADER
}
else
{
  echo"incorrect password matchup please try again";
}
?>
User avatar
Obadiah
Forum Regular
Posts: 580
Joined: Mon Jul 31, 2006 9:13 am
Location: Ashland, KY
Contact:

Post by Obadiah »

sweet!...works like a charm...thanks for the help guys :)
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Post by Mordred »

This is okay for the SQL injections, but there is still the problem with duplicated passwords

Code: Select all

UPDATE customer SET password = '$nPassword' WHERE password = '$sPassword'
I'm tired of having Obadiah try this and that and work out why it happens :P (hey, man, less WoW, more PHP, that's the spirit!) so here's the answer:

This code will change the password of ALL PEOPLE who have the same old password. The password IS NOT an unique identifier of the user accounts. I suggest you add an autoincrement column, get it in the select statement, and then use it in the update statement as an identifier (i.e. the where clause should be like WHERE id='$id')
User avatar
Obadiah
Forum Regular
Posts: 580
Joined: Mon Jul 31, 2006 9:13 am
Location: Ashland, KY
Contact:

Post by Obadiah »

adding another clause in there to make it so that it will only change that users name wouldent be difficult at all....

and all non-players of WoW shall either join my obsession or perish at the will of the forsaken
FOR THE HORDE!!! :twisted: :lol:

i went with this...it was something i should have done earlier on

Code: Select all

$sql = "UPDATE customer SET password = '$nPassword' WHERE password = '$sPassword' AND user_name= '{$_SESSION['logname']}'" ;
since my username is the PK then it works pretty well as far as the duplicate passwords go
Post Reply