2

The assignment is to create a login system secured for SQL injections and XSS.

It's in PHP and i'm using PDO with prepared statements obviously. Which from my point of view should protect againts the simplest injections at least. The ones I'm throwing at the system are ineffectual.

If you believe it's not secure againts SQL injection I would gladly take comments and an example of what should work against it.

With regards to XSS I haven't done anything to protect againts it yet. From the school I believe it was mostly about html or url-encode the inputs. But would that be enough? From google I also have some articles about sql escaping.

So again, comments or help on XSS protection could be nice, or is it only a matter of escaping the inputs before it reaches the sql query?

The code:

function Log_in($username, $password) {
$db = new PDO("host;dbname", "user", "password");
$login = $db -> prepare("SELECT * FROM Users WHERE Username =:username and Password =:password");
$login->bindValue(':username', $username, PDO::PARAM_STR);
$login->bindValue(':password', $password, PDO::PARAM_STR);
$login->execute();

global $count;
$count = $login->rowCount();

global $data;
$data=$login->fetchColumn(2);
return $count;
return $data;
}

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

if (isset($_POST['submit'])) {
    getSalt($username);
    foreach ($rowset as $row) {
        $salt = $row[0];        
    }

    $hashpassword = sha1($password.$salt);
    Log_in($username, $hashpassword);
    if($count==1){
        session_start();
        $_SESSION['email'] = $data;
        header("location:index.php");
    }   else {
        echo "Wrong";
    }
}
gerre
  • 155
  • 1
  • 11
  • 4
    This should be on codereview. – Daan Apr 21 '15 at 14:42
  • You're using prepared statements and placeholders. Therefor your query, as written above, is invulnerable to direct sql injection. But that doesn't mean the REST of your system is. you can't just declare one tiny little portion of a codebase secure and say "that means everything else is secure". – Marc B Apr 21 '15 at 14:45
  • Code review; that or closed as a duplicate to the "How to avoid SQL injection" link. http://stackoverflow.com/q/60174/ – Funk Forty Niner Apr 21 '15 at 14:47
  • As noted above, this REALLY should be on codereview.stackexchange.com since you are not trying to solve a specific problem. – Mike Brant Apr 21 '15 at 14:50
  • 4
    I'm voting to close this question as off-topic because this should be on codereview.stackexchange.com – Mike Brant Apr 21 '15 at 14:51
  • Sorry. I simply wasn't aware of the codereview site. I'll try post it there then. And obviously I'm not saying just because this part is secure then everything else is too. But it's this part that I'm suppose to do and secure here. – gerre Apr 21 '15 at 14:55
  • 2
    For password storage, 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). `sha1` isn't the best anymore. You'll probably be told the same thing on code review; *I hope*. – Funk Forty Niner Apr 21 '15 at 14:56

0 Answers0