Link Details

Link 86778 thumbnail
User 261337 avatar

By Rocky1138
via johnrockefeller.net
Published: Jun 13 2008 / 13:53

How to secure your php site from MySQL injection attacks.
  • 13
  • 5
  • 1917
  • 612

Comments

Add your comment
User 297562 avatar

Sven Arild Helleland replied ago:

0 votes Vote down Vote up Reply

The topic is important but there is several errors in the first part of the article so its obvious the author does not really know what hes talking about other than repeating what hes read on other webpages.

User 300356 avatar

exsecror.pip.verisignlabs.com replied ago:

0 votes Vote down Vote up Reply

I have to say I agree with svenarild, the author of this article lacks any real experience with either SQL or PHP to properly write an article on protection against SQL Injections. There was a lot of bad/improper information and methods.

User 261337 avatar

John Rockefeller replied ago:

0 votes Vote down Vote up Reply

I appreciate both of your replies but I'm curious: Where in my blog post was there an error? I also have to correct exsecror: I do have "real experience" with SQL and PHP. Perhaps my article does not correctly show my level of skill in these two subjects, but I am definitely not a newbie. Finally, I would like to ask that if there were "bad/improper information and methods" that you post them here so the general public can benefit from the knowledge of how to correctly prevent MySQL injection attacks.

Thanks!

User 297562 avatar

Sven Arild Helleland replied ago:

1 votes Vote down Vote up Reply

Sure John, here is a few errors and issues from your article.

The first issue is your injection example, if you take a closer look you will notice it would not work. In the query you md5 hash the password field, so the "injection" would be hashed into a 32 character long string. You also might want to use an example closer to a real world situation, no one checks for the username and password at once in a query anymore as it makes it difficult to log attempts. Also if you show hashing of a password at least add in a salt.

The second error is that you mention "Even more unscrupulous users (read: the real jackasses) could send in multiple queries including DELETE queries." even through you mention MySQL all over the article. MySQL does not permit query stacking nor does it allow executing multiple queries in a single function call. If you try this, the entire query fails. I.e. "user'; DELETE FROM tbl_Users;" will not work on MySQL even if your able to inject it properly (Keep in mind that this is an important point, so it should be mentioned. But you should let your readers know its not an issue with MySQL).

For escaping the values, mysql_real_escape_string() works; you can also cast any bool/integer/float values. But you should really use prepared statements instead, either PDO or mysqli (note that PDO has its weaknesses, so if you use stored procedures in MySQL that return multiple columns you are better off with mysqli).

In your final point, number three. You mention proper MySQL syntax; there is no need to use the reverse apostraphe at all in queries. The only reason its there is so you are able to use reserved words as table and column names. Though you should never use reserved words for that anyway. For the single quotes, you should always use it around fields expecting a string/character. For values (int/float) you dont need to use them. (Note, the query you used as the first example would fail as a username usally contain characters as well)

I know my reply might seem a little harsh, but I personally feel the issues I mentioned are major ones that you should have noticed when reviewing the article before posting it as they are imo very obvious. Though I applaud you for asking what the issues are, I hope you will update your article to reflect what I mentioned above. If you need anyone to proof read the updated version, let me know.

User 261337 avatar

John Rockefeller replied ago:

0 votes Vote down Vote up Reply

Hi Sven, thanks for your reply.

I will definitely take you up on your offer to proof the revised version . The things you mention in your reply are things that would very much help to improve my article and indeed the level of detail required when preventing mysql injection attacks. I have spent a bit of time researching the subject and have never really been satisfied with what's out there. I guess in my attempt to improve that it ended up being "more of the same." So, I would like to take the opportunity to write a more comprehensive article based off of the information in your reply.

How do I contact you with my updated article?

User 261337 avatar

John Rockefeller replied ago:

0 votes Vote down Vote up Reply

Hate to reply to my own post, but I've updated the article to take advantage of your generosity. Please take a look and let me know if you want to yell at me some more :)

User 294742 avatar

dmitryx replied ago:

-2 votes Vote down Vote up Reply

here we go.
d in dzone is goind to stand for dumb-people.

User 292525 avatar

scriptone replied ago:

0 votes Vote down Vote up Reply

yeah, the d in dimistry as well (check out this site if you don't believe me http://dmitryx.com/)

User 274601 avatar

webid replied ago:

2 votes Vote down Vote up Reply

finally some productive 'discussion' in the comments!

Dmitryx: only one D? so humble

User 297562 avatar

Sven Arild Helleland replied ago:

1 votes Vote down Vote up Reply

John:
Sorry for the delay, did not notice you had posted a comment.

There is only thing that you should consider changing and that is:

Moving the prepared statements under step 1, or move it to step 2.

The key with prepared statements is that you do not need to escape the value with mysql_real_escape_string() etc. As they are escaped automatically when you bind the value. With other words, you should make it clear that they can use either one, but that they should not use both at the same time.

I would also recommend that you add PDO as an option in the prepared statments text. Unless your using advanced stored procedures, its a very good choice as well.

Other than this the article looks ok. There is a few things that I would have written different, but there is no issues or errors in the content; so I would say its good to go :)

dmitryx:
Got to love your positive thinking :)

User 261337 avatar

John Rockefeller replied ago:

0 votes Vote down Vote up Reply

Thanks Sven, I will implement your changes. Thank you again for the great feedback.

Dmitryx: I realize that my original post may not have been the most informative or intelligent article in the world, but thanks to some of the feedback I've received here on Dzone I've managed to improve both my techniques as well as the article I wrote. Isn't that the point of this whole endeavor? To be a better coder in the evening than you were in the morning? I think so.

Thanks to all who have helped and replied.

User 294742 avatar

dmitryx replied ago:

-2 votes Vote down Vote up Reply

Well, this way you wont be any better.
The right thing to do is open php manual and read it. Just one or two pages, no big deal. And there is also google, with millions articles on SQL injection. And sure, people who find this article any good had never read official manual. Well, bad for them. What kind of programmer any person is, if he or she doesnt even know things written on top of the manual?
http://php.net/results.php?q=injection&p=manual&l=en

Add your comment


Html tags not supported. Reply is editable for 5 minutes. Use [code lang="java|ruby|sql|css|xml"][/code] to post code snippets.