Login system problems

[taigah50]

Below is my login script.

<?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:2 in /home/taigah/public_html/members/login.php on line 43

Any solutions?

[lemon-tree]

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]

Some things

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

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

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

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

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

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

$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
}

//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);"

$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

$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
}

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

[taigah50]

Right, thanks. I've started a PDO version of my login.

[lemon-tree]

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]

Until my PDO code is complete, I've decided to use MySQL.

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

$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]

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.

[taigah50]

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
    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.

[misson]

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]

Below's my latest revision, and it still gives me the same parse error, but on a different line.

<?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.