What's wrong with this login code? SQL and PHP

Question

What's wrong with this login code? the same code worked in a different website that I programmed

this is the login form:

<html>
<head>
<meta http-equiv="content-type" content-type="text/html; charset=windows-1255">
<title>היכנס!</title>
</head>
<body dir="rtl">
<center>
<form action="loginaction.php" method="post">
שם משתמש:<input type="text" name="username" value=""><br>
סיסמה:<input type="password" name="password" value=""><br>
<input type="submit" value="היכנס!"><br>
</form>
<br>
<?php
$error=$_GET['error'];
if ($error==1) {
    echo "שם המשתמש או הסיסמה לא נכונים!";
}
;
?>
</center>
</body>
</html>

this is the login action code:

<?php

include('config.php');

$username=$_GET['username'];
$password=$_GET['password'];
$userpassword=md5($password);

$checkquery=mysql_query("SELECT * FROM `users` WHERE `username`='$username' AND `password`='$userpassword'");
if (mysql_num_rows($checkquery)>0) {
    $row=mysql_query("SELECT `userid` FROM `users` WHERE `username`='$username' AND `password`='$userpassword'");
    $data=mysql_fetch_array($row);
    $userid=$data['userid'];
    setcookie("userid", $userid);
    setcookie("username", $username);
    setcookie("userpassword", $userpassword);
    echo "<script type=\"text/javascript\">window.location='index.php';</script>";
} else {
    echo "<script type=\"text/javascript\">window.location='login.php?error=1';</script>";
}
?>

here is the config.php:

<?php

$link=mysql_connect("mysql9.000webhost.com", "a2803040_dbase", "I won't publish the password here"); 
mysql_select_db("a2803040_dbase", $link);
mysql_set_charset("utf8", $link);

?>

Please help me ASAP! As I said, I tried the same code in a different website that I programmed and it worked

Additional Comment : Please start using PDO. http://stackoverflow.com/questions/601300/what-is-sql-injection — Duikboot, Oct 25 '13 at 12:08
There is a lot of problems here, use of `md5`, `mysql_*` functions which are deprecated, use of cookies to store login information, SQL injection vulnerabilities... — naththedeveloper, Oct 25 '13 at 12:09
`login.php?username=admin' --`. Done - now I'm logged in as admin - thanks for the SQL injection! I'd be very interested in knowing the URL where you did this before. — h2ooooooo, Oct 25 '13 at 12:12
[**Please, don't use `mysql_*` functions in new code**](http://bit.ly/phpmsql). They are no longer maintained [and are officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). See the [**red box**](http://j.mp/Te9zIL)? Learn about [*prepared statements*](http://j.mp/T9hLWi) instead, and use [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli) - [this article](http://j.mp/QEx8IB) will help you decide which. If you choose PDO, [here is a good tutorial](http://j.mp/PoWehJ). — h2ooooooo, Oct 25 '13 at 12:13

score 6 · Answer 1 · answered Oct 25 '13 at 12:09

6

Do you mean, "Why won't it log me in", or "Why is this a horrible piece of insecure code that a 2-year-old could crack, and I'm lucky my client doesn't sue me"?

Go read something about "SQL Injection" for starters, then PDO, then use an authentication library made by someone with many years of experience in this sort of thing

answered Oct 25 '13 at 12:09

jmadsen

3,635
2
33
49

Or would you just tell me why it's not secured and tell me how to change it so it would work and will be secured? – Hagaymosko Oct 25 '13 at 12:11
@user2918521 You don't seem to be sanitizing user input (never trust input from user, as it may be harmful). You store password in "plain text" (md5 hashing is pretty much like storing it in plain text), if someone get access to your server's database (which is very likely), like even you/not to be trusted admin, then they could get all the user's passwords. The list goes on... :p – Horse SMith Oct 25 '13 at 12:12
No - go learn how to be a programmer by yourself, then ask questions. I told you what subjects to start reading – jmadsen Oct 25 '13 at 12:15
Let's not forget that you can get anyones MD5 password if they're logged in (just read the cookies that are being sent on every single request - all it takes is a simple man-in-the-middle attack or physical access!). – h2ooooooo Oct 25 '13 at 12:17

score 2 · Answer 2 · answered Oct 25 '13 at 12:06

2

you posted your for via POST method but you are trying to GET it , change

$_GET['username'];

with

$_POST['username']; or $_REQUEST['username'];

answered Oct 25 '13 at 12:06

user2092317

3,226
5
24
35

WHAT IS THE ERROR YOU ARE GETTING /? – user2092317 Oct 25 '13 at 12:09

Raul Luna · Answer 3 · 2013-10-25T12:27:51.380

For god sake!!!

As other have said, SQL Injection is one of the problems. Don't even try to put this crap into production.

What will happen if the user enters as username ' = '' or '' = '???

The select query issued to the server will be:

SELECT * FROM users WHERE username='' or '' = '' AND password='whatever you want'

hence returning true and therefore, allowing the user to enter into the application.

You must use mysql_escape_string for make ALL the sentences in your code, but specially the login and those related with security (checking permissions and things like that).

$safe_username = mysql_escape_string( $username ) 
$safe_password = mysql_escape_string( $userpassword )
$checkquery=mysql_query("SELECT * FROM `users` WHERE `username`='$safe_username' AND `password`='$safe_password'");

This will solve your problem because in this case the select issued to the server would be:

SELECT * FROM users WHERE username='\' or \'\' = \'' AND password='whatever you want'

And it will look correctly for the username ' or '' = ' in the database.

What's wrong with this login code? SQL and PHP

3 Answers3