Avoid SQL injection in registration form php

Question

I've a simple registration form on my localhost (still testing), and I am wondering if it can be attacked by SQL injection?

Code:

$name = mysql_real_escape_string($_POST['name']);
$password = mysql_real_escape_string($_POST['password']);
$password = md5($password);
$email = mysql_real_escape_string($_POST['email']);
$refId = $_GET['refid'];
$ip = $_SERVER['REMOTE_ADDR'];


$add = mysql_query("INSERT INTO `users` (`name`, `password`, `email`, `refId`, `ip`)
VALUES('$name','$password','$email','$refId', '$ip')") or die(mysql_error());

Is that safe or can someone use SQL injection (I'd like to know how)? How can I avoid injection?

HAve you [read this](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) already? prepared statements, and to be as safe as possible: use master and slave DB's, stored procedures and, if needs must, parse the string with your own function — Elias Van Ootegem, Jul 10 '12 at 17:00
Avoiding injection with the mysql_ library requires you to do tedious stuff like this for every single query, and even then you're not guaranteed to be safe. The standard way to avoid injections is with prepared queries, which tell the db engine that it simply must not treat the string you pass as anything other than an argument in the query you specified. PHP has two libraries for this, mysqli and PDO. google them and read up on prepared queries. It takes a little more work to set up, but it's worth it. — octern, Jul 10 '12 at 17:00
Stored procedures aren't necessary to avoid SQL injection, although that would work; prepared statements are the easiest approach, or an ORM like Doctrine or Propel. — halfer, Jul 10 '12 at 17:03
What type of SQL? The specific solution to injection differs from implementation to implementation (though the general principles are the same). — RBarryYoung, Jul 10 '12 at 17:03
possible duplicate of [Am I safe from SQL-injections?](http://stackoverflow.com/questions/5990100/am-i-safe-from-sql-injections) — halfer, Jul 10 '12 at 17:29

score 6 · Answer 1 · edited May 23 '17 at 10:28

The best way to avoid injections is to use Prepared Statements.
For prepared Statements I prefer to use PDO to handle all my DB stuff. here is some PDO sample code I wrote to retrieve some basic login information:

$sql=new PDO("mysql:host=127.0.0.1;dbname=name","user","password");
        $user=$_POST[user];

        $query="select Salt,Passwd from User
                where Name=:user";
        $stmt=$sql->prepare($query);
        $stmt->bindParam(':user',$user);
        $stmt->execute();
        $dr=$stmt->fetch();        
        $sql=null; 
        $password=$_POST[pass];
        $salt=$dr['Salt'];

... etc

Read this page for more information on PDO. If you want to know what each line of code here is doing, read this answer I gave to another post.

Well if they all have the same salt, it kind of defeats the purpose ;) — CountMurphy, Jul 10 '12 at 17:17

score 5 · Answer 2 · edited Jul 10 '12 at 17:07

5

It is indeed vulnerable; you haven't escaped $_GET['refid']. Take this URL for example:

yourpage.php?refid='%2C'')%3B%20DROP%20TABLE%20users%3B--

Avoiding SQL injection is easy. Use prepared statements and PDO. For example:

$query = $dbh->prepare("INSERT INTO `users`(`name`, `password`, `email`, `refId`, `ip`)
VALUES(:name, :password, :email, :refId, :ip)");
$query->bindValue(':name', $_POST['name']);
$query->bindValue(':password', md5($_POST['password'])); # Do not use MD5 for password hashing, and especially not without salt.
$query->bindValue(':email', $_POST['email']);
$query->bindValue(':refid', $_GET['refid']);
$query->bindValue(':ip', $_SERVER['REMOTE_ADDR']);
$query->execute();

edited Jul 10 '12 at 17:07

halfer

19,824
17
99
186

answered Jul 10 '12 at 17:00

Seconded on never use Md5, even with a salt. At the least use something like sha2, at the most use PBKDF2 – CountMurphy Jul 10 '12 at 17:12
1

@CountMurphy: Personally, I would recommend bcrypt - and not only at most, either. – Jul 10 '12 at 17:13
forgot about bcrypt. Good call. – CountMurphy Jul 10 '12 at 17:15

score 4 · Answer 3 · answered Jul 10 '12 at 17:02

4

You didn't mysql_real_escape_string the $refId, even though that's grabbed from $_GET. That basically makes all your other injection prevention measures moot. On a tangent, if you escape string the password before md5-ing, it changes the hash.

answered Jul 10 '12 at 17:02

Palladium

3,723
4
15
19

Avoid SQL injection in registration form php

3 Answers3

Linked