Is my code safe from SQL injection? Simple Sessions login

Question

This is my first login system using SESSIONS. Just wondering, if it's safe for SQL injection?

<?php
$username = $_POST['username'];
$password = $_POST['password'];

if(isset($username, $password)) {
    if(get_magic_quotes_gpc()) {
        $ousername = stripslashes($username);
        $uusername = mysql_real_escape_string(stripslashes($username));
        $opassword = stripslashes($_POST['password']);
    } else {
        $uusername = mysql_real_escape_string($username);
        $opassword = $password;
    }   
    $req = mysql_query('select password,id from users where username="'.$uusername.'"');
    $dn = mysql_fetch_array($req);

    if($dn['password']==$opassword and mysql_num_rows($req)>0)
    {
        $form = false;
        $_SESSION['username'] = $_POST['username'];
        $_SESSION['userid'] = $dn['id'];        

    echo 'Logged in m8';
    } else {
        $form = true;
        $message = 'The username or password is incorrect.';
    }
} else {
    $form = true;
}
if($form)
{ 
if(isset($message)) {
    echo '<div class="message">'.$message.'</div>';
}           
?>

I was getting a SQL injection before on my highscores script, so I was wondering if my simple sessions login script have any, I'm 40% there is one.. What usually causes SQL injections? Thanks!

[SQL injection that gets around mysql_real_escape_string()](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) — John Woo, Feb 22 '13 at 03:07
It is. But using [PDO with prepared statements](http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php) is much easier than mysql_ babysitting and manual escaping. — mario, Feb 22 '13 at 03:09
I don't get the difference between PDO and mysql_, really, what is the main reason to use it ? I call mysql at connect.php (I use require for the file) — Jony kale, Feb 22 '13 at 03:15
Magic quotes, mysql_* functions.... Please use PDO. It is 2013 now. Let's embrace the new improved more secure apis. — itachi, Feb 22 '13 at 03:16
`mysql_*` is deprecated and will get remove. Why you want to insist continuing deprecated functions? — itachi, Feb 22 '13 at 03:18
**Either its safe or not it wont work in php 5.5 ...** however you can use [this function form here in your common sense's answer](http://stackoverflow.com/a/14112684/1723893) its pretty much same what pdo does and safe too — NullPoiиteя, Feb 22 '13 at 03:18
Well I find PDO pretty hard, but thanks Ill try it out - get used to it. — Jony kale, Feb 22 '13 at 03:33
Implicit in this code is that you're storing your passwords plaintext; you shouldn't be doing that. At minimum, you should be hashing your passwords with a salt. — Yahel, Feb 22 '13 at 03:43
See http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php?lq=1 (relinked). If you don't know it's safe, it's not. Stop wasting time :( — , Feb 22 '13 at 05:09
`*sigh*` magic quotes are real and a developer *have* to take care of them. PDO and prepared statments aren't silver bullet, they have no placeholder for identifiers for example. mysql will be present on shared hostings for at least 5 years. What we really need is to get rid of **whatever** raw API calls in our code, not of mysql_* only. It is not API but only knowledge that can make your code secure - one can write insecure code with PDO as well. But who cares on this God blessed site... — Your Common Sense, Feb 22 '13 at 05:32

score 1 · Accepted Answer · edited May 23 '17 at 12:12

Although your code is okay, your idea of protection is wrong
mysql_real_escape_string does not protect from injections. It does format strings. As long as you have your strings properly formatted, they are safe.

The problem begins when you're trying to use the same function to format non-strings - a numbers for example.
It become totally useless and your SQL - vulnerable.

So, you can keep with your current code but in the future, as soon as you will need to use another query part - you will need to format it differently. Here is a set of full rules: In PHP when submitting strings to the database should I take care of illegal characters using htmlspecialchars() or use a regular expression?

And of course do not escape password! If i have a password like wef5623'sdf - it will never let me in!

By the way, I have no idea why you're using SO much variables for just a single value - $_POST['username'], $username, $uusername, $ousername - what's all this?

+1 this answer is informative and makes sense ... – NullPoiиteя Feb 22 '13 at 05:44 — NullPoiиteя, Feb 22 '13 at 05:44

score -3 · Answer 2 · answered Feb 22 '13 at 03:34

-3

i would change $opassword = $password;

to

 $opassword = mysql_real_escape_string($password);

sql injection can be done via password fields too.

answered Feb 22 '13 at 03:34

Lee Ginger-Ninja McCarthy

186
3
4
14

they don't use password in the SQL. So, no SQL injection via password field. – Your Common Sense Feb 22 '13 at 05:09
-1 of course..agreed with(@YourCommonSense) i don't think anyone uses plan text in password isnt it ? i dont know how one can use sql injection by [`sha1("' or '1'='1' /* '");`](http://codepad.viper-7.com/KRsckw) – NullPoiиteя Feb 22 '13 at 05:51

Is my code safe from SQL injection? Simple Sessions login

2 Answers2