-1

I tried to write a simple registration and log-in code but I get a problem in the log-in file.

login.php -

<?php
session_start();
include"header.php";
include"config.php";
connect();

if(isset($_POST['login'])){
    if(isset($_SESSION['uid'])){
    echo"you are already logged in";
    }else{

    $username = protect($_POST['username']);
    $password = protect($_POST['password']);


    $login_check = mysql_query("SELECT `id` FROM `users` WHERE `username` = '$username' AND     `password` = '".md5($password)."' ") or die (mysql_error());
    echo $login_check;
    if(mysql_num_rows($login_check) == 0){
        echo"invalid login password";
    }else{
    $get_id = mysql_fetch_assoc($login_check);
    $_SESSION['uid']=$get_id['id'];
    header("Location: main.php");   
    }

    }

    }else{
    echo"u need to log in";
    }


/* */

include"footer.php";
?>

When I try this code there are no errors but I always get: "invalid login password". If I try to select id from users just by username it works fine and gets me to main.php.

Here is my Config.php and form.

config.php -

<?php

function connect(){
mysql_connect("localhost","root","");
mysql_select_db("game2");
}

function protect($string){
        return mysql_real_escape_string(strip_tags(addslashes($string)));
}

?>

Form from header.php :

<form action="login.php" method="POST">
Username:<input type="text" name="username"/><br />
Password:<input type="password" name="password"/><br />
<input type="submit" name="login" value="login"/>
</form>

Update: So I followed Marc B answer and i think there must be something wrong with my register.php file. First I noticed that whatever i would write into the password input the hashs for every ID in the table were the same. So I even cut off protect() and md5 parts and i saw that whatever i would write as password there will be blank spot in the password in the table. Username and email goes into the table just fine. register.php -

<?php
session_start();
include"header.php";

?>

<?php
include("config.php");
connect();
?>
Rejestracja:
<br />
<?php

if (isset($_POST['register'])){
    $username = protect($_POST['username']);
    $password = protect($_POST['password']);
    $email = protect($_POST['email']);

    if($username=="" || $password="" || $email == ""){
        echo"Wypelnij wszystkie pola";
    }elseif(strlen($username) > 20){
        echo"Nazwa musi miec max 20 znaków !";

    }elseif(strlen($email) > 50){
        echo"Email musi miec max 50 znakow!";
    }else{
        $register1 = mysql_query("SELECT `id` FROM `users` WHERE `username`='$username'") or die(mysql_error());
        $register2 = mysql_query("SELECT `id` FROM `users` WHERE `email`='$email'") or die(mysql_error());
        if(mysql_num_rows($register1) > 0){
            echo"Jest juz taki uzytkownik!";
        }elseif (mysql_num_rows($register2) > 0){
            echo"email jest juz zajety";
        }else{
            $insertstats = mysql_query("INSERT INTO `stats` (`gold`,`attack`,`defense`,`food`) VALUES (100,5,5,100)") or die(mysql_error()) ;
            $insertunits = mysql_query("INSERT INTO `units` (`worker`,`farmer`,`warrior`,`defender`) VALUES (5,5,5,10)") or die(mysql_error()) ;
            $insertweapons = mysql_query("INSERT INTO `weapons` (`sword`,`shield`) VALUES (5,5)") or die(mysql_error()) ;
            $insertusers = mysql_query("INSERT INTO `users` (`username`,`password`,`email`) VALUES ('$username','".md5($password)."','$email')") or die(mysql_error()) ;

            echo"Zarejestrowales sie";
        }
    }


}

?>
<br />
<form action="register.php" method="POST">
Nazwa uzytkownika:<input type="text" name="username"/><br />
Haslo:<input type="password" name="password"/><br />
E-mail:<input type="text" name="email"/><br />
<input type="submit" name="register" value="Register" /><br />


</form>
<?php
include"footer.php";

?>

Okay i found my mistake... if($username=="" || $password="" || $email == "") im so silly. Thanks for all the suggestions about improving the code. Sorry for troubles.

jenio
  • 3
  • 2
  • 1
    Add error reporting to the top of your file(s) right after your opening ` – Funk Forty Niner Sep 25 '14 at 18:51
  • 2
    I suggest you not use MD5 for password storage. It's old and considered broken. Use [**CRYPT_BLOWFISH**](http://security.stackexchange.com/q/36471) or PHP 5.5's [`password_hash()`](http://www.php.net/manual/en/function.password-hash.php) function. For PHP < 5.5 use the [`password_hash() compatibility pack`](https://github.com/ircmaxell/password_compat). – Funk Forty Niner Sep 25 '14 at 18:52
  • 1
    **Danger**: You are using [an **obsolete** database API](http://stackoverflow.com/q/12859942/19068) and should use a [modern replacement](http://php.net/manual/en/mysqlinfo.api.choosing.php). You are also **vulnerable to [SQL injection attacks](http://bobby-tables.com/)** that a modern API would make it easier to [defend](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) yourself from. – Quentin Sep 25 '14 at 18:52
  • 1
    Further reading for Fred's comment about [password hashing](http://php.net/manual/en/faq.passwords.php). – Quentin Sep 25 '14 at 18:53
  • check if the coloumn which has your password doesn't have an empty space at the starting because your code seems ok !! – Uzumaki Naruto Sep 25 '14 at 18:58

1 Answers1

2

You're not comparing your passwords properly. You're sql escaping them THEN doing the md5 hash, which means any escapes added to the password will become part of the hash.

e.g.

$foo = 'password with " quote';
$hashed_foo = md5($foo);         // e938052104fd22cbdf0c822065557f09
$escaped_foo = addslashes($foo);
$hashed_escaped_foo = md5($foo); // b144a3e7ad3ea3fd4ae6ade0aa49a9b9

Note how the two hashes changed. If you manually md5'd the original password, then you'll never be able to get a match since your escaping is CHANGING the original string, before it gets hashed, which means the hash will always be different than the original un-escaped string's.

Marc B
  • 356,200
  • 43
  • 426
  • 500