Login system problems

taigah50

New Member
Messages
11
Reaction score
0
Points
0
Below is my login script.

PHP:
<?php	
	$username = $_POST['username'];
	$password = $_POST['password'];
	
	//connect to the database here
	
	if($_GET['action'] == "login")
{
	$dbhost = 'localhost';
	$dbname = 'taigah_members';
	$dbuser = 'taigah_th100';
	$dbpass = 'taigaisb';

	$conn = mysql_connect($dbhost, $dbuser, $dbpass);
	mysql_select_db($dbname, $conn);
}

	//sanitize username
	$username = mysql_real_escape_string($username);
	
	$query = "INSERT INTO users ( username, password, salt )
			VALUES ( '$username' , '$hash' , '$salt' );";
	mysql_query($query);
		
	$username = mysql_real_escape_string($username);
	
		$query = "SELECT password, salt
		FROM users
		WHERE username = '$username';";
	$result = mysql_query($query);
	
	if(mysql_num_rows($result) < 1) //no such user exists
{
		header('Location: login_form.php?login=failed&cause='.urlencode('Invalid User/Password'));
		die();
}
	
	$userData = mysql_fetch_array($result, MYSQL_ASSOC);
	$hash = sha1( $userData['salt'] . sha1($password) );
	
	if($hash != $userData['password']) //incorrect password
{
		header('Location: login_form.php?login=failed&cause='.urlencode('Invalid User/Password'));
		die();
}

	//Login successful; redirect to another page or display "login success" message
	session_register("$username");
	session_register("$password");
	header('Location: index.php');
	
	mysql_close();
	
?>

When it tries to redirect to index.php, THIS happens to it.

Warning: mysql_real_escape_string() [function.mysql-real-escape-string]: Access denied for user 'taigah'@'10.33.248.75' (using password: NO) in /home/taigah/public_html/members/login.php on line 28

Warning: mysql_real_escape_string() [function.mysql-real-escape-string]: A link to the server could not be established in /home/taigah/public_html/members/login.php on line 28

Warning: mysql_query() [function.mysql-query]: Access denied for user 'taigah'@'10.33.248.75' (using password: NO) in /home/taigah/public_html/members/login.php on line 32

Warning: mysql_query() [function.mysql-query]: A link to the server could not be established in /home/taigah/public_html/members/login.php on line 32

Warning: mysql_real_escape_string() [function.mysql-real-escape-string]: Access denied for user 'taigah'@'10.33.248.75' (using password: NO) in /home/taigah/public_html/members/login.php on line 34

Warning: mysql_real_escape_string() [function.mysql-real-escape-string]: A link to the server could not be established in /home/taigah/public_html/members/login.php on line 34

Warning: mysql_query() [function.mysql-query]: Access denied for user 'taigah'@'10.33.248.75' (using password: NO) in /home/taigah/public_html/members/login.php on line 39

Warning: mysql_query() [function.mysql-query]: A link to the server could not be established in /home/taigah/public_html/members/login.php on line 39

Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in /home/taigah/public_html/members/login.php on line 41

Warning: Cannot modify header information - headers already sent by (output started at /home/taigah/public_html/members/login.php:28) in /home/taigah/public_html/members/login.php on line 43

Any solutions?
 

lemon-tree

x10 Minion
Community Support
Messages
1,420
Reaction score
46
Points
48
Well, it's telling you pretty clearly there that there is something wrong with your SQL connection. Looking at the code I see that you have wrapped your connect code in an if statement, so if you haven't set the GET data then no connection will be made. I recommend you look into transitioning over to using PDO for your database connections, it is both more secure and has more advanced query features.

Additionally, at no point in your code have you created the $hash and $salt variables for your password and also I don't see why you are doing an insert of that data in the login command. When creating the hash, ensure you use one of the more recent hash algorithms (SHA256 etc) with a different salt for every user to prevent dictionary attacks.
 

bdistler

Well-Known Member
Prime Account
Messages
3,534
Reaction score
196
Points
63
Some things

PHP:
$username = $_POST['username'];
$password = $_POST['password'];

You need to test both of these for what they are or are not

PHP:
if($_GET['action'] == "login")

what to do if $_GET['action'] != "login" also use "===" for this test

PHP:
$dbhost = 'localhost';
$dbname = 'taigah_members';
$dbuser = 'taigah_th100';
$dbpass = 'taigaisb';

This stuff should be in a 'include' file above you root folder

PHP:
$conn = mysql_connect($dbhost, $dbuser, $dbpass);
mysql_select_db($dbname, $conn);

This is where most of your errors start - you should test

if (!$conn)
{
do something about the error
}

if (!mysql_select_db($dbname, $conn))
{
do something about the error
}

PHP:
//sanitize username
$username = mysql_real_escape_string($username);

before you use "mysql_real_escape_string"
test if "Magic Quotes" is set on
(a server at x10Hosting had it set on)

if (get_magic_quotes_gpc())
{
undo the effects of "Magic Quotes" on $_POST[$username]
before mysql_real_escape_string($username)
}

you do not need it after "mysql_query($query);"

PHP:
$query = "INSERT INTO users ( username, password, salt )
        VALUES ( '$username' , '$hash' , '$salt' );";
mysql_query($query);

if you do this now then your test "if(mysql_num_rows($result) < 1) //no such user exists"
will all-ways be true
as lemon-tree said you never set $hash or '$salt

PHP:
$query = "SELECT password, salt FROM users WHERE username = '$username';";

The end of this line ==>';";<== S/B ==>'";<==

after the query test the result

if (!$result)
{
do something about the error
}

PHP:
session_register("$username");

use "Sessions"

NEVER store users name and password

take the user's name and password add in some salt pull a hash on the mix
save the hash in the data base
then in this script (at login) take name and password mix in the salt pull a hash
look for the hash in the database

be sure to use a strong password on your data base and for the FTP to your site

I agree with lemon-tree take a look at using PDO for your database connections
but what you have started will work
 
Last edited:

lemon-tree

x10 Minion
Community Support
Messages
1,420
Reaction score
46
Points
48
Right, thanks. I've started a PDO version of my login.
Good to hear. Six months down the road you'll be thanking yourself for taking the time now to do this, when your code is more advanced and caters for a larger site.
 

taigah50

New Member
Messages
11
Reaction score
0
Points
0
Until my PDO code is complete, I've decided to use MySQL.

Anyway, here's an excerpt from my revised code:

PHP:
$username = $_POST['username'];
$dbhost = 'localhost';
$dbname = 'taigah_members';
$dbuser = 'taigah_th100';
$dbpass = 'taigaisb';
$conn = mysql_connect($dbhost, $dbuser, $dbpass);
mysql_select_db($dbname, $conn);
$username = mysql_real_escape_string($username);
$query = "SELECT password, salt FROM users WHERE username = '$username'";
$result = mysql_query($query);
$userData = mysql_fetch_array($result);
$hash = sha1( $userData['salt'] . sha1($password) );
if($hash != $userData['password']) //incorrect password
{
header('Location: login_form.php?login=failed&cause='.urlencode('Invalid User/Password'));
die();
}
mysql_close();

This gives the following parse error:
Parse error: syntax error, unexpected T_IF in /home/taigah/public_html/members/login.php on line 54

Well... since the above code is just an excerpt, the error is at "if($hash != $userData['password']) //incorrect password". Did I miss a mistake?
 

misson

Community Paragon
Community Support
Messages
2,572
Reaction score
72
Points
48
The fragment you posted has no syntax error. Remember, sample code should be representative of the actual code (it should also be complete, which the sample is not). Code (both sample and production) should also be indented consistently according to some style, which will make some mistakes more obvious.

As for the error, one possibility is that the line before the "if" is missing a terminating semicolon.
 
Last edited:

taigah50

New Member
Messages
11
Reaction score
0
Points
0
The fragment you posted has no syntax error. Remember, sample code should be representative of the actual code (it should also be complete, which the sample is not). Code (both sample and production) should also be indented consistently according to some style, which will make some mistakes more obvious.

As for the error, one possibility is that the line before the "if" is missing a terminating semicolon.

PHP:
<?php
	session_start();
	
	$username = $_POST['username'];
	$password = $_POST['password'];
	
	//connect to the database here
	
	$dbhost = 'localhost';
	$dbname = 'taigah_members';
	$dbuser = 'taigah_th100';
	$dbpass = '*******';

	$conn = mysql_connect($dbhost, $dbuser, $dbpass);
	mysql_select_db($dbname, $conn);
	
	if (!$conn)
{
	mysql_close();
	header('Location: login_form.php?login=failed&cause='.urlencode('Database error.'));
	die();
}

	if (!mysql_select_db($dbname, $conn))
{
	mysql_close();
	header('Location: login_form.php?login=failed&cause='.urlencode('Cannot connect to database.'));
	die();
}
	//sanitize username
		
	$username = mysql_real_escape_string($username);
	
		$query = "SELECT password, salt
		FROM users
		WHERE username = '$username'";
	$result = mysql_query($query);
	
	if(!$result)
{
	header('Location: login_form.php?login=failed&cause='.urlencode('Database is unavailable.'));
	die();
}
	
	if(mysql_num_rows($result) < 1) //no such user exists
{
		header('Location: login_form.php?login=failed&cause='.urlencode('Invalid User/Password'));
		die();
}
	
	$userData = mysql_fetch_array($result);
	$hash = sha1( $userData['salt'] . sha1($password) );
 
	if($hash != $userData['password']) //incorrect password
{
	header('Location: login_form.php?login=failed&cause='.urlencode('Invalid User/Password'));
	die();
}

	//Login successful; redirect to another page or display "login success" message
	header('Location: index.php');
	
	mysql_close();
	
?>

Can't see any syntax errors, either. And the semicolons are in their rightful places.
 
Last edited:

misson

Community Paragon
Community Support
Messages
2,572
Reaction score
72
Points
48
Are you sure that code generates the syntax error you mentioned?

As a link in my previous post explains, example code should also be concise, which benefits you just as much as us, for various reasons. It's especially helpful when it comes to syntax errors. It's still not self-contained and, if it doesn't generate the error, it's not representative.

That better not be your real DB password I see.

As lemon-tree points out, SHA1 is considered broken by security experts. You can call other hash functions with hash. If there are any real users in your database, you can process them by adding a new column to indicate which users have been updated (or haven't been updated, or which hash was used to hash the password). When a user successfully logs in that hasn't been processed, re-hash their password and update the DB.
 

taigah50

New Member
Messages
11
Reaction score
0
Points
0
Below's my latest revision, and it still gives me the same parse error, but on a different line.

PHP:
<?php
	$username = $_POST['username'];
	$password = $_POST['password'];
		
	function validateUser()
{
	session_regenerate_id (); //this is a security measure.
	$_SESSION['valid'] = 1;
	$_SESSION['userid'] = $userid;
}
	//connect to the database here
	
	$dbhost = 'localhost';
	$dbname = 'taigah_members';
	$dbuser = 'taigah_th100';
	$dbpass = '********';

	$conn = mysql_connect($dbhost, $dbuser, $dbpass);
	mysql_select_db($dbname, $conn);
	
	if (!$conn)
{
	mysql_close();
	header('Location: login_form.php?login=failed&cause='.urlencode('Database error.'));
	die();
}

	if (!mysql_select_db($dbname, $conn))
{
	mysql_close();
	header('Location: login_form.php?login=failed&cause='.urlencode('Cannot connect to database.'));
	die();
}
	//sanitize username
		
	$username = mysql_real_escape_string($username);
	
		$query = "SELECT username, password, salt
		FROM users
		WHERE username = '$username';";
		
	$result = mysql_query($query);
	$userData = mysql_fetch_array($result, MYSQL_ASSOC);
	
	if($username != $userData['username']) //no such user exists
{
		header('Location: login_form.php?login=failed&cause='.urlencode('Invalid User/Password'));
		die();
}

	$hash = md5( $userData['salt'] . md5($password) );
*
	if($hash != $userData['password']) //incorrect password
{
	header('Location: login_form.php?login=failed&cause='.urlencode('Invalid User/Password'));
	die();
}

	validateUser();
	
	//Login successful; redirect to another page or display "login success" message
	
	mysql_close();
	
	header('Location: index.php');
?>

I am clearly sure that the code above has correct syntax. I also converted from sha1 to md5 - this might be the problem.
 

lemon-tree

x10 Minion
Community Support
Messages
1,420
Reaction score
46
Points
48
I also converted from sha1 to md5 - this might be the problem.
Why would you do that? That is the equivalent of removing all the mediocre locks from your doors at home and pretty much just leaving the doors wide open. MD5 is renowned to be relatively easy to find collisions if you can throw a little computing power at it, so it shouldn't be used to store a password. SHA1 has been shown to have collisions too and, whilst being considerably better than MD5, is not suggested for use anymore either.
We recommended that you move to a MORE secure algorithm, not a less secure. In any case, here is what you should change it to as a bare minimum:
PHP:
//Change this line:
$hash = md5( $userData['salt'] . md5($password) );
//To this:
$hash = hash_hmac('sha512', $password, $userData['salt']);
//You could also use SHA256 or SHA384 if you want a shorter hash length with no current loss of security.
Essentially, try to avoid using md4, md5 or sha1 for passwords in any scripts you write. One thing you should always remember is to NEVER assume security by obscurity, i.e don't think that because your site is small that nobody will try to hack it and let your security methods slacken as a result.

As for the syntax error, I'll take a look now.
 
Last edited:

bdistler

Well-Known Member
Prime Account
Messages
3,534
Reaction score
196
Points
63
I do not see ==>session_start()<==
before ==>session_regenerate_id<==

what is ==>$userid<==

after
PHP:
$result = mysql_query($query);

I would add
PHP:
if (!$result)
{
do your error thing
}

then to debug
PHP:
print "array is \$userData";
print "<pre>";
print_r($userData);
print "</pre>";
EXIT;

I also would set my own error thing
PHP:
error_reporting(0);
$old_error_handler = set_error_handler("userErrorHandler");

function userErrorHandler ($errno, $errmsg, $filename, $linenum,  $vars)
{
  my error way
}
 
Last edited:

misson

Community Paragon
Community Support
Messages
2,572
Reaction score
72
Points
48
Below's my latest revision, and it still gives me the same parse error, but on a different line.
Which line? The line after the "*", where you compare the stored and supplied password hashes? (If that's an indicator, it's not at all clear. If that's what you have in your source code, the "*" there will cause a syntax error.) You should indicate this clearly in the code using a comment. The more we have to guess at, the less likely we are to be able to come up with correct (or any) answer.

You're still not posting concise, self-contained sample code. Here's the link again: "Short, Self Contained, Correct (Compilable), Example". Sample code should be just enough to be complete . The extra code in your posts makes it harder to zero-in on the problem. It also makes the sample not self-contained.

Read my sig, then apply the directions to my previous posts. You've got some reading to do.

I also converted from sha1 to md5 - this might be the problem.
That's changing the semantics of the program. Changing function names won't cause a syntax error.
 
Top