SQL Injection in this code?

Question

I have used RIPS to test my code for SQL injection threats. And this is one of them that has come up:

    Userinput reaches sensitive sink due to insecure usage of mysql_real_escape_string() without quotes (Blind exploitation)
370: mysql_query mysql_query("INSERT INTO `members` (`Username`, `Password`, `Name`, `AccessLevel`, `LastLogin`) VALUES ('" . mysql_real_escape_string($user) . "', '" . mysql_real_escape_string($pass) . "', '" . mysql_real_escape_string($fuln) . "', '" . mysql_real_escape_string($al) . "', '" . mysql_real_escape_string($now) . "');"); 
364: $user = htmlentities($_POST['user'], ENT_QUOTES); 
365: $pass = htmlentities(md5($_POST['pass']), ENT_QUOTES); 
366: $fuln = htmlentities($_POST['fuln'], ENT_QUOTES); 
367: $al = htmlentities($_POST['al'], ENT_QUOTES); 
368: $now = time(); 
requires:
359: if($action == "newadmin")
360: if($memberinfo['AccessLevel'] == 3)
363: if($_GET['submit'] == "new")
369: if($user && $pass && $fuln)

Could someone please explain to me why this code would be vulnerable to SQL injection? I think it is either because of lack of knowledge or it is a false positive. I have been researching but I cannot find out anything new and I seem to be going in circles.

Intead of mysql use PDO or MYSQLI with prepare statements :) — Perry, Oct 01 '13 at 09:37
The password could do with a bit of per-database and per-use salting, too. — halfer, Oct 01 '13 at 09:45
[this link](http://stackoverflow.com/questions/5414731/are-mysql-real-escape-string-and-mysql-escape-string-sufficient-for-app-secu) has answer to your question. — Dharmesh Patel, Oct 01 '13 at 09:50

neelsg · Accepted Answer · 2013-10-01T10:09:09.493

1

This has to be a false positive. The issue described is when you use mysql_real_escape_string(), but you don't ensure that the result is enclosed in quotes. I checked and your strings are properly enclosed.

You should however note that the mysql API is deprecated as of PHP 5.5.0 and you should be using mysqli or PDO MySQL instead.

EDIT: Also note that as per the comments, using md5 is not secure at all and that you should salt it by concatenating the password to a secret code before generating the hash. If you salt it, an intruder cannot simply use a pre-built dictionary of hashes to get the passwords, they would need to create a new dictonary specifically with your salt included

edited Oct 01 '13 at 10:09

answered Oct 01 '13 at 09:45

neelsg

4,802
5
34
58

This absolutely obliterates my spare time over the next week, but sounds like a necessity. Thank you very much for the assistance! – Sidupac Oct 01 '13 at 11:10
Managed to completely update the website to mysqli and add salting, didn't take as long as expected! Thanks again! – Sidupac Oct 01 '13 at 13:39

SQL Injection in this code?

1 Answers1