Is this a secure way to structure a mysql_query in PHP

Question

I have tried and tried to achieve an SQL injection by making custom queries to the server outside of firefox.

Inside the php, all variables are passed into the query in a string like this.

Note, by this stage, $_POST has not been touched.

mysql_query('INSERT INTO users (password, username) VALUES(' . sha1($_POST['password']) . ',' . $_POST['username'] . '));

Is that a secure way to make a change?

Answer 1

You should definitely escape the username with mysql_real_escape_string.

Of course the best solution would be to use prepared statements. That way the separation of query syntax and data is made on the mysql API level.

And, as others pointed out, values should absolutely be surrounded with quotes. Especially the text ones.

Answer 2

Look at http://php.net/mysqli.real-escape-string, adding that to all incomming values should help making it safer.

Answer 3

what you are doing there is dangerous since someone can send a POST request with an evil user name. you can either check every parameter and escape it, additionally you could use mysqli (http://php.net/manual/en/book.mysqli.php), and bind the parameters using prepare+bind.

the first step is good to avoid exploits on other users, while the second is good for your server side safety.

also check out this question: http://stackoverflow.com/questions/47087/how-do-you-prevent-sql-injection-in-lamp-applications

Answer 4

this is insecure, unless magic_quotes_gpc configuration directive is turned on.
var_dump(ini_get('magic_quotes_gpc')); or phpinfo(); can show you the actual value

Note that this directive is dirty, deprecated and all-hated. And will make some passwords not work.

Answer 5

On checking your code I'm surprised it works at all when you don't quote the literals you are inserting - you will be generating code like:

INSERT INTO user (password, username) VALUES (abc1234fg00000, admin);

So it will give an error every time. Assuming this is just a typo....

The mysql extension limits your ability to perform injection attacks by only allowing one query per call. Also, there is limited scope for an injection attack on a INSERT statement. Add to that the fact that you change the representation to a neutral format before splicing into the insert statement means that it is not a potential avenue for such an attack. However, your code should fall over if someone POSTs a username containing a single quote (if it doesn't then you've got magic_quotes enabled enabled which is deprecated).

OTOH if you apply the same method to validating the account then you are wide open to injection attacks - consider

"SELECT 1 
FROM users
WHERE username='" . $_POST['username'] . "'
AND password='" . sha1($_POST['username'] . "';";

If $_POST['username'] contains "admin' OR 1 " then your system is compromised.

You should always use mysql_real_escape_string() unless you've made the data safe using a different function (e.g. sha1, bas64_encode....but NOT addslashes)

C.

Answer 6

I wonder sometimes if they'll find me in the old folks home, screaming "BIND VARIABLES" at the nurses as they walk by.

It's 2010 people, not 1995.

BIND VARIABLES!

BIND VARIABLES!