Sql injections

PHP
1

hi all,
i have done simple login form with username=“admin” and password=“admin123”.
i am checking for sql injections.i have given “mysql_real_escape_string”
for both username and password fields.
but also it is not working…
if i give username as “admin --” and click the submit button(not giving password also) it is taking to the next page…
tell me whats wrong in my below code…


<?php
session_start();
mysql_connect("localhost","root","");
mysql_select_db("test");
if(isset($_POST['sub']))
{
$username=mysql_real_escape_string($_POST['txtuname']);
$password=mysql_real_escape_string($_POST['txtpwd']);
$check=mysql_query("SELECT DISTINCT `username`,`password` FROM `log` WHERE `username`='$username'") or die("Error: " . mysql_error());
while($find = mysql_fetch_array($check)) 
 {
 list($username,$output) = $find;
 }
if($password==$output) 
 { 
$_session['si']=session_id();
echo "<script> location='view1.php'</script>";
 }
else
echo "invalid";
}
?>
<table width="200" height="150" bgcolor="lightblue" border="1" align="center">
<tr><td style="font-size:25;color:red" align="center" colspan="2">Login Form </td></tr>
<form method="post" action="">
<tr><td align="right" width="100">
Username:</td><td><input type="text" name="txtuname" </td></tr>
<tr><td align="right" width="100">
Password:</td><td><input type="password" name="txtpwd" </td></tr>
<tr><td align="right" width="100">
<input type="submit" value="login" name="sub" </td></tr>
</form>
</table>
2

‘password’ is a reserved mySQL keyword. It won’t work. Change it to passwd or something other than password. I also think you’re overcomplicating the code. This is basically what I do.

$username=mysql_real_escape_string($_POST['txtuname']);
$password=mysql_real_escape_string($_POST['txtpwd']);

$result = mysql_query("SELECT * FROM log WHERE username='$username' AND passwd='$password'");

if(mysql_num_rows($result) > 0) {
	// user checks out
} else {
	// user doesn't check out
}

You shouldn’t have to check for DISTINCT because you should never have more than one instance of a username. Your username field should be UNIQUE so that each user gets a distinct username at signup. If you don’t do that you’re asking for trouble. :slight_smile:

3

if($password==$output)

if your query returns nothing, $output is not set.
if your password is empty, $password == $output is TRUE.

Change your query to check the database WHERE username = $username AND password = $password.
If it finds a result, you’ve got a valid login.

BTW: I would alter your system to encrypt that password, for safety.

4

Not true. Password will work, you just HAVE to enclose it in backticks (`) or it will fail.

SELECT password FROM table // Bad

SELECT `password` FROM `table` //Good
5

It depends on the database you use. I just use ‘passwd’ and avoid any trouble, especially if I want my code to be portable.

But you’re correct, he is using mySQL. Thanks for the heads up. :slight_smile: