I'm developing a website and I'm trying to secure the connection part.
I used the addslashes
function on $login
to stop SQL injection but some friends told me that's not enough security. However, they didn't show me how to exploit this vulnerability.
How can I / could you break this code?
How can I secure it?
<?php
if ( isset($_POST) && (!empty($_POST['login'])) && (!empty($_POST['password'])) )
{
extract($_POST);
$sql = "SELECT pseudo, sex, city, pwd FROM auth WHERE pseudo = '".addslashes($login)."'";
$req = mysql_query($sql) or die('Erreur SQL');
if (mysql_num_rows($req) > 0)
{
$data = mysql_fetch_assoc($req);
if ($password == $data['pwd'])
{
$loginOK = true;
}
}
}
?>
You should use mysql_real_escape_string for escaping string input parameters in a query. Use type casting to sanitize numeric parameters and whitelisting to sanitize identifiers.
In the referenced PHP page, there is an example of a sql injection in a login form.
A better solution would be to use prepared statements, you can do this by using PDO or mysqli.
You are storing your passwords in plaintext! That's a major security issue if ever I saw one. What to do about that: at least use a (per-user) salted hash of the password, as seen e.g. here.
Use:
mysql_real_escape_string($inputToClean);
There's another gaping security hole - extract
. It may save you from typing a few characters, but opens up holes too numerous to mention, for it will overwrite any global variables.
What happens if I post this?
$_POST {
'login' => 'Admin',
'loginOK' => 1
}
Guess what, $loginOK is now == 1 , and I'll be logged in as Admin.
Save yourself a lot of grief later, and just use the variables you want to use, instead of relying on the horrible hack that is extract
.
Apart from the usage of addslashes()
, these are some random issues found in this code:
isset($_POST)
is always TRUE
, unless you run it from the command line. You can probably remove it.
empty()
is very tricky. For instance, if $password = '0'
then empty($password)
is TRUE.
- You can do this:
if( isset($_POST['login']) && $_POST['login']!='' ){}
extract($_POST)
is a huge vulnerability: anyone can set variables in your code from outside.
$password == $data['pwd']
suggests that you are storing plain text passwords in your database. That's a terrible practice. Google for "salted password".
- You can also do
$loginOK = $password == $data['pwd'];
. Do you realise why? ;-)
Rather than addslashes
you should use mysql_real_escape_string
.