-5

I am developing a system for a client. I need some suggestions for that. Is this a safe code to prevent sql injection?

$username=$_POST["username"];
$password=md5($_POST["password"]);

$num_rows=mysql_num_rows(mysql_query("select * from table where username='$username' AND password='$password'"));

if($num_rows>0)
{
    echo "Logged in";
}
else 
{
    echo "Incorrect username or password";
}

Thanks in advance for your suggestions. Please elaborate your answer why the above code is unsafe.Thanks please dont negative vote the question if you dont have any answer.

amrit
  • 15
  • 7
  • Stop using deprecated `mysql_*` functions. Use MySQLi / PDO instead. – Raptor Oct 15 '14 at 05:48
  • your script is open for SQL Injections do some validations before adding user inputs in MySQL query and `mysql` extension is deprecated use `mysqli` OR `PDO` (PHP Data Objects) , – Azeem Hassni Oct 15 '14 at 05:54
  • can you provide any sql injection which can satisfy this condition? – amrit Oct 15 '14 at 06:05
  • Check the duplicate link, and 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) for password hashing. MD5 is old and considered broken. – Funk Forty Niner Oct 15 '14 at 07:28

2 Answers2

2

No, the code is unsafe for SQL Injection, especially $username variable. You should escape the variable, via mysql_real_escape_string(). If I were you, I will add trim() function to both user name and password, as users sometimes add spaces before / after text input accidentally.

More importantly, Stop using deprecated mysql_* functions. Use MySQLi / PDO instead.

Note: $password is fine, as it's generated by md5() hash, which won't contain any character that leads to SQL error / injection.

Raptor
  • 53,206
  • 45
  • 230
  • 366
0

the upper code is not satisfying any of the sql injections like 

' or '1'='1' -- 
' or '1'='1' ({ 
' or '1'='1' /*

how u can say that it is open for sql injections.can you elaborate your answers friends?

amrit
  • 15
  • 7
  • this is not an answer. Please use **edit** button in your question. – Raptor Oct 15 '14 at 06:03
  • yes this is not an answer friend,but this is answer for those who r saying the above code is open for sql injection :) – amrit Oct 15 '14 at 06:06
  • sql injections are those queries which will always return some results from the database by satisfying the condition like $a='abc OR 1=1 ' username=$abc. as 1 is always equals to 1 so it will return some positive results. – amrit Oct 15 '14 at 06:15
  • bro i know wat is safe and unsafe,please read my question i m asking,how u can say the above code is unsafe,thats it. – amrit Oct 15 '14 at 06:19
  • I just put your code in a file and, aside from the fact that I had to change table to `table` in order to make it a valid query, the first line you have in your "answer" will log you in to any user perfectly fine. If you don't know what you're doing, I'd highly recommend using an authentication system someone else wrote rather than leaving huge security holes in your client's system. If this is a personal project, great, but please don't try to develop production code similar to this. – Ryan Pendleton Oct 15 '14 at 06:35
  • Ryan how can u login without having valid username and password in the above query and script?can u explain? – amrit Oct 15 '14 at 11:21