My inject filter

erikhillis

My sites authentication system was recently bypassed. Will this code stop sql injects on my login page?

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

   $username_fixed1 = str_replace("'","", $username);
   $username_fixed2 = str_replace(")","", $username_fixed1);
   $username_fixed3 = str_replace("(","", $username_fixed2);
   $username_fixed4 = str_replace("IS NULL; --","", $username_fixed3);
   $username_fixed5 = str_replace("--","", $username_fixed4);

$pass = MD5($password);

$sql = mysql_query("SELECT * FROM user WHERE username='$username_fixed5' AND password='$pass' AND activated='1'");

Will this stop the bad guys from hacking into my site via my login form?

MarkR

The only way of doing this correctly is using the correct SQL escaping function for your database. In this case, this will probably be mysql_real_escape_string.

Don't bother with any other method as it either
a) Won't work
or
b) Will strip out legitimate characters.

Personally I suspect your solution of both of these.

Mark

Weedpacket

After seconding MarkR (if you want to write reactive code that reliably predicts what attacks will be made, you first have to become omniscient), I'll address a few coding-level issues that have relevance elsewhere:

$POST[password] should be $POST['password']; likewise for username.
(And development should be performed on an installation configured with error reporting set to E_ALL. This would have caught the above.)
There is no check that $_POST['password'] even exists before it's used. (Maybe that happens earlier)
It is completely unnecesssary to use a new $username_fixed variable every time: unless you want to use the value of $username_fixed3 for something afterwards, it's just going to be sitting around taking up space.
[man]str_replace[/man] can take arrays in its arguments.
Do you really need every column from the user table, no matter what they are (you already know the values of username, password, and activated)? If you don't, then "Select *" is a bit sloppy (it might hurt database query optimisation).