Sql injection fix?

[jrws]

Hey guys, I have a problem. I made a simple login script but it won't protect against sql injections. I changed it so that it would echo the username in the login form, and it echoed:

\' OR\'x=x

 

Here is the code I used (Blanked out password and user)

<?php
/* Login Page
User can login here, if they cannot, displays a register page. As soon as they login they are redirected to welcome.php
Has the added functionality to make a cookie, from the remember me button 
/*/
session_start();
$host = '****';
$user = '***';
$password = '';
$database = 'site';
$con = mysql_connect($host, $user, $password) or die('Error: Cannot Connect');
$condb = mysql_select_db($database);
function safe($string)
{
    if (get_magic_quotes_gpc()) {
        $string = stripslashes($string);
    }
    $string = mysql_real_escape_string($string);
    $string = strip_tags($string);
    return trim($string);
}
function encrypt($string)
{
    $string = md5(sha1($string));
    return $string;
}


if (isset($_POST['login'])) {
    $user = safe($_POST['user']);
    $pass = safe($_POST['pass']);
    $pass1 = encrypt(safe($_POST['pass']));
    if(!ctype_alnum($user) OR !ctype_alnum($pass)){
	return $errorA;
} 
    $error = array();
    $q = mysql_query("SELECT * FROM user WHERE username = '$user' AND password = '$pass1' LIMIT 1") or
        die('Unable to connect...');
    if (!$q OR $errorA) {
        $error[] = 'Appologises, username or password does not exist or are incorrect...</span>';
    }
    if (count($error) > 0) {
        echo "The following errors occured:</br>";
        foreach ($error as $err) {
            echo "<span style=\"color: red;\">$err<br>";
        }
        echo 'Click to go <a href="#" onClick="history.go(-1)">Back</a>';
    } //else {
       // if (isset($_POST['remember'])) {
           // setcookie("cookname", $_SESSION['username'], time() + 60 * 60 * 24 * 100, "/");
          //  setcookie("cookpass", $_SESSION['password'], time() + 60 * 60 * 24 * 100, "/");
        //} //else {
          //  $_SESSION['username'] = $user;
          //  $_SESSION['password'] = $pass;
      //  }
    echo "<meta http-equiv=\"Refresh\" content=\"0;url=welcome.php\"> Going to welcome page<br>Click <a href=\"welcome.php\">here</a> if you are not automatically redirected.";
    //}
} else {
?>
<form action="<? $_SERVER['PHP_SELF'] ?>" method="post">
Username:<input type ="text" name= "user"><br>
Password:<input type ="password" name= "pass"><br>
<input type="submit" name = "login" value = "Login">
<input type="checkbox" name="remember">
<font size="2">Remember me next time
</form>

<?
}

mysql_close();
?>

To see what I mean in a live example go to (my) site and login only as 'OR'x=x and it will auto redirect to welcome page...

http://sandstorm.net46.net

[PFMaBiSmAd]

The problem is not really sql injection. The problem is that your code is just testing if the query executed without error. After you check if the query executed without error, you must check if the number of rows found is equal to one.

 

You can in fact enter just about anything for a user/password and your code will redirect to the welcome page (give it a try.)

[jrws]

Yes, that is what I was talking about. But unfortunately my limited capacity of knowledge of PHP refrains from allowing me to work the solution completely by myself. So if anyone my help, I would be appreciative. Thanks

[CroNiX]

This is what you want to use to see if you got a result:

http://us.php.net/manual/en/function.mysql-num-rows.php

[jrws]

Thank you. I will try a new code.