Small problem with PHP code?

Community Forums/General Help/Small problem with PHP code?

Guy Fawkes(Posted 2012) [#1]
Hi all. As you may or may not know, I've been working on an "ETnA" login system the last few weeks. I've gotten it nearly working perfectly, EXCEPT for the check to see if the username and password entered, ACTUALLY DO exist in the database, and that it IS THEIR username they are using. Now... The PROBLEM, I have described IN the "//" marks, in the code below.

Just incase, I'm going to make it easier to read:



The above is the problem I'm having.

Code:



The above isnt the ACTUAL code, but a mockup OF the original ETNA code, to see if I can get it to print the arrays of the usernames and passwords, and compare them with the "If" statement, with the typed out username and password, to see if they exist, and return a 1 depending on if they typed the right username and password.

Any help is greatly appreciated!

Thank You! :)

Last edited 2012


Foppy(Posted 2012) [#2]
I am not sure what the problem is (where does it go wrong?) but I think there should not be a ; at the end of your SQL query.

$sql="SELECT user, passw FROM etna_userbase WHERE user='" . $username . "' AND passw='" . $password . "'";

// note: ; removed and using $password instead of $pass

Also, you could use the MD5 version of $password directly in the query, but I suppose this is just elaborate test code of yours.

Last edited 2012


Sledge(Posted 2012) [#3]
//If the typed out Username IS equal to ONE OF THE Usernames in the database,
//AND the MD5() OF the typed out Password IS equal to 1 of the MD5() Passwords already
//in the Database
, then return answer with a 1, and send back to program using ETNA
//Else, send a 0 back to the program using ETNA

//Code ***MUST*** do this for instance if a user typed in a password that IS in the database,
//BUT it is NOT THEIR password, then answer should return a 0

I'm curious about the bit I bolded in the quote. Why would you ever check to see if the digest from the submitted password matched ANY of those in the database? Rather than, y'know, the password digest for the particular username in question?


GfK(Posted 2012) [#4]
I am not sure what the problem is (where does it go wrong?) but I think there should not be a ; at the end of your SQL query.
Yea, that should be there. in PHP a semicolon just indicates the end of a line (since PHP stuff can span multiple lines). The only exception is when there is only a single function call encapsulated within PHP tags; i.e. <?php echo "Hello" ?>, is valid.

[edit] Didn't realise your code was a corrected version - thought you were posting a copy of his for reference.

Last edited 2012


Guy Fawkes(Posted 2012) [#5]
Well, what I do is, with ETNA, I have the user type in the NON-digest version of the username and password, and then have it sent Encrypted using ETNA's Encrypt function, and then check the digest of the typed password, with the digest that's already in the database, then send a 1 or 0 back, depending on if it's the right digest in the array of digests or not.... Problem is... I cant get it to work EXCEPT if the user types the username and password the 1st time, COMPLETELY correct... and that is why I built this test code. I just don't see what's going wrong with it...


Foppy(Posted 2012) [#6]
in PHP a semicolon just indicates the end of a line (since PHP stuff can span multiple lines).

Indeed in PHP there should be a semicolon, but in this case there is also a semicolon in the string that is the SQL query.

Although when I do a search for that in Google, it seems that in some variations a semicolon is required, so it depends on which system is used.


Derron(Posted 2012) [#7]
I'm not in the mood to correct your mysql code directly - as this part is of security concerns and if you don't know what you do - it potentially harms all of your clients ... let it do someone who knows what to do!


if your game client is not able to split user and password:
(1.1) send: md5(user . password)
(1.2) generate hash: $sentHash = md5(sentVariable.SALT);
(1.3a) SELECT * FROM mytable WHERE hash='".$sentHash."'
(1.3b) if no results and in registration mode: insert fields into your DB: sentUser/"user", sentHash/"hash"
(1.4) check if results are there (affected_rows if inserted or count of resultset)

in that case you can't do nick-changes without resetting the password too.
--

if your game is able to split user and password:
(2.1) send user, md5(password)
(2.2) generate hash: sentHash = md5(user.password.Salt)
(2.3a) SELECT * FROM mytable WHERE user='".$sentUser."' AND hash='".$sentHash."'
(2.3b) if no results and in registration mode: insert fields into your DB: sentUser/"user", sentPasswort/"password", hash/"hash"
(2.4) check if results are there (affected_rows if inserted or count of resultset)

in that case individual nick or password changes are possible without affecting each other - you can calculate the hash on your server:
case "nickchange":
$nick = mysql_real_escape_string(isset($_GET["newnick"]) ? $_GET["newnick"] : $oldNickFromDB);
$newHash = md5($nick.$oldPassFromDB.SALT);

UPDATE mytable SET nick="'.$nick.'", hash="'.$newHash.'" WHERE nick="'.$oldNickFromDB.'" AND ...

and so forth.


Common mistakes in Select-Statement are missing "AND"-connections and wrong table-layout.

bye
Ron

Last edited 2012


Guy Fawkes(Posted 2012) [#8]
I'm using ETNA. Not actual PHP to do all this. I'm only using PHP to communicate with ETNA. And thanks, but the above doesn't help me in the least. I want to use my OWN script, in which I know MOSTLY what I'm doing in it.

Last edited 2012


Derron(Posted 2012) [#9]
If it doesnt help you... the problem of user authentification lies in your logic.


to find out wether user data is already in your database you need a exclusively used condition.
In most cases it is the accountname. (you could also do hardware-hashes mixed with registrationtimestamps ... but then you will have not 100% guarantee of exclusive data).


Ok i finally read your code... like said: your logic has some mistakes in it - rethink wether you need a double check of account-data
(1) SELECT ...WHERE XYZ - already filters your dataset
(2) WHILE - you are checking the resultset again versus your filters ... for what?

some changes and comments:

<?php
	$username	= mysql_real_escape_string($user); // <-- source of $user?
	$password	= mysql_real_escape_string($pass); // <-- source of $pass?

	$sql		= "SELECT user, passw FROM etna_userbase WHERE user='" . $username . "' AND passw='" . $pass . "'";
			
	//echo $sql."<br/>\r\n";
			
	$result = mysql_query($sql) or die('ERROR: '.mysql_error());
			
	//echo $result."<br/>\r\n";
			
	//init not needed in your case... if used outside "while" -> $ans1 = (!isset($ans1)) ? array() : $ans1;
	//$ans1 = array();
	//$ans2 = array();

	while($row = mysql_fetch_array($result)){
		/*
			hmpf ... for what are you checking the resultset?
			you ALREADY FILTERED using WHERE...
			so ALL data in this loop has user=$user and pass=$pass

		$answer1 = $row['user'];
		$answer2 = $row['passw'];
		
		$ans1[] = $answer1;
		$ans2[] = $answer2;
		
		//If the typed out Username IS equal to ONE OF THE Usernames in the database,
		//AND the MD5() OF the typed out Password IS equal to 1 of the MD5() Passwords already
		//in the Database, then return answer with a 1, and send back to program using ETNA
		//Else, send a 0 back to the program using ETNA

		//Code ***MUST*** do this for instance if a user typed in a password that IS in the database,
		//BUT it is NOT THEIR password, then answer should return a 0
		
		if($user == $ans1[0] && MD5($pass) == $ans2[0]){ $answer = 1; } else { $answer = 0; }
		*/

		//doing THIS is enough to store found accounts
		//in most cases you end up with 1 account - else your db data is corrupted
		$foundAccounts[] = $row;
	}

	print "given username: ".$user."<br />";
	print "given passwordHash: ".$password."<br /><br />";

	if(isset($foundAccounts))
		foreach($foundAccounts as $acc) {		
			print "account data:<br /><pre>". print_r($acc,1)."</pre><br />";
		}
	else
		print "<b>no accounts found</b>";
?>


check the data your script received ($user and $pass).
For first tries - and if unsure - do not md5 the data. send plain unencrypted data.
If then working - add encrypting.

DON'T FORGET to hash with salt -> md5(username.'mySecret§?$%CODE')
Although rainbowtables can be generated if your Binary gets analysed (like reverse engineering or stringsearch). but this takes a while and I don't think some "leet cheater kids" are willed to do so.
If you really get it way safer: use sha instead of md5 (but thats overkill to implement by yourself).

bye Ron

Last edited 2012

Last edited 2012


Guy Fawkes(Posted 2012) [#10]
Thanks anyway, but I finally got it sorted earlier! :)