Through PHPDeveloper I came across a Devshed article related to SQL Injection.
The one major flaw in the article is that it is suggested input validation is enough protection. This is not the case. Lets start with this example (literal copy-paste):
- <?php
-
- //Validate text input
- if (! preg_match('/^[-a-z.-@,'s]*$/i',$_POST['name']))
- {
- } else
- if ($empty==0)
- {
- }
- else
- {
- }
- ?>
Ignoring the syntax errors, I believe the author here actually intended to allow the usage of the single quote character ('). This will allow sql injection in a lot of queries.
The worst part is the following:
Want stronger protection? If you need stronger protection you can validate the user input using the above scripts andmysql_real_escape_string; this will offer secondary protection in case the above validation scripts fail due to some reason. Discussing this feature is beyond the scope of this article and you can read useful resources on:http://www.php.net/mysql_real_escape_string
However, before you can use this feature, you must be connected to a MySQL database, or else it will return an error. Some really talented hackers can play around with mysql_real_escape_string, which is why it is highly recommended to have a double filter in your scripts (validating scripts +mysql_real_escape_string) to make hacking much more difficult.
Here it is is suggested that mysql_real_escape_string should be used only in the event somebody feels they need 'more' protection. Also, I would like to meet these talented hackers.
What you really should do
- Always use mysql_real_escape_string. Escaping and validating serve two (important) purposes, and validating does not take away the need to escape. mysql_real_escape_string escapes many more 'bad' characters and it works with different collations.
- Blacklisting 'bad' things is in most cases a bad idea. Always try to use whitelists for acceptable input.
- Don't write security related articles if you don't really know the subject. You are potentially placing people at risk, and you promote bad habits.
You're missing the most important piece that trumps input validation and escaping altogether:
YOU NEED TO PARAMETERIZE YOUR QUERIES.
*sigh*
I personally don't used prepared statement as much.. I prepare them myself (so to speak ;). I definitely feel its a very good habit though.
thanks!
The quality of PHP articles on Devshed is generally very low, I can recall several instances where I have had to shake my head at content in their PHP section.
Which is sad as not only does it spread horrible info, but it also degrades the reputation of the other articles submitted. You'd think they had someone with just a hint of knowledge read over the articles.
What do you mean... "I personally don't used prepared statement as much.." ??
You're still relying on your input validation to stand up when faced with unexpected input. Why not just do it the right way and parameterize your queries?
This blog post isn't much better than the article you link to. Good job.
I continually find it highly interesting that they do not have editors around this type of content on much more high profile sites. Especially when dealing with programming tutorials it is especially bad.
However, to comment on the usage of mysql_real_escape_string, there is also many more things that people should be aware of such as character encoding and making sure your client connects as the same encoding that you are sending it :)
Marcin,
We use vsprintf which gives us the benefits of escaping parameters, but we don't use mysql's prepared statements. There were some bugs in the past related to query caches not kicking in, so at that point we decided to rely on vsprintf + mysql_real_escape_string. The biggest difference is that values still need quotes around them, and the different syntax.
Do you think I might be making a mistake there?
Thanks!
Evert
Is it April 1st yet? ;)
There should have been an editor at the helm. The article is one of the worst for such a public site.
Evert, we all know for a fact that "using vsprintf" instead of doing the right thing is a mistake.
While it's good that you saw the other guy's mistake, the real lesson here is that you're correct: security related articles should only be written by experienced people.
In any case, if you can't rely on your database API to get queries right, then you've got much bigger things to worry about. Assuming you're correct, then the inability to use prepared queries means that the solution is to use a real database, because you're already doomed. If you're wrong, then you don't know enough to write your query creating code anyway.
Either way, you need to use a quality database (PostgreSQL is also open source and is well respected, and SQL Server Express is free and more than good enough for most people's needs) and not screw around building queries in ASCII strings.
Still, the other possibility is that I'm entirely retarded, and that leads us back to where we started: taking security advice from anonomous tards on the net (such as me) is usually a bad idea. ;)
But I've never seen a professional security expert advocate any form of SQL injection prevention that doesn't include the use of prepared queries.
I definitely see the drawback of encoding the values to ascii and letting mysql's query parser decoding it again..
What I'm wondering is: Are there known problems to using mysql_real_escape_string today, or is this a best practice because its easier to make mistakes using plain queries.
I thought it would be the latter, so although I see the benefit it wasn't high on the priority list..
I'm definitely curious about the general opinion. I've seen enough security-related articles actively use plain queries in examples.
Yes, mysql_real_escape_string does not offer FULL protection.
Just think about %LIKE for a second. mysql_real_escape_string excapes ' and " and various other chars, but not %. I wish I still had the bookmark, but I've seen people attack scripts that use mysql_real_escape_string using %LIKE before.
As well as addslashes ( which you should know is like mysql_real_escape_string, only weaker ). On certain queries, I use sprintf, mysql_real_escape_string AND addcslashes to excape the % and _ . I'm sure its not 100%, but it holds up extremely well so far.
You can't stop all attacks, and lucky for you ( and the rest ), you will eventually be hacked. Instead of being pissed at all the awesome damage caused, check your log files and you will learn first hand about security ( rather than learning from others ).
Also, avoid the i modifier in Regex, you should all know that by now...
interesting. I was of the (obviously misguided) opinion that correctly using a web framework like CodeIgniter, Kohana or the godawful Zend Framework, would help prevent most injection attacks. ah well, "wait for attack and study the log files" seems much more fun!
dscn: the codeigniter documentation advises you to write prepared statements: http://codeigniter.com/user_guide/database/queries.html <-- at="" the="" bottom="" ;).="" and="" there="" is="" also="" a="" "security"="" chapter="" in="" documentation="" where="" they="" mention="" database="" queries.="" i'm="" not="" using="" codeigniter="" btw.="">
Indeed. Although why you'd be writing queries manually when using such a framework is beyond me.
Anyhow, that's not really what I was getting at.. The guys above seemed to be saying that it doesn't matter what you do, you will eventually be hacked anyway - which I don't see as a terribly useful way to look at it :)
wtf! it's too early for april fools day!
Writing yet another blog article about mysql_real_escape_string() doesn't actually solve the problem. The root problem lies with using string concatenation for building SQL queries at all.
If you do so, sooner or later, one occourence is going to slip through. String escaping is too easily forgotten. Exhibit A: Wordpress.
If you want to write PHP professionally, just fucking use prepared statements, PDO, bind parameters. Real solution.
> I personally don't used prepared statement as much..
I don't use prepared statements either, so I don't see the problem? If you have your own solutions to the sql injection problem then so be it, so long as your solution has been extensively tested.
As for the site, I stop going there years ago as most of the articles were pretty basic, for the uneducated and as you've pointed out, complete and utter horse manure.
Nothing has changed much since I see.
Les,
We do have a well tested solution, so that's also where I was coming from. I'm definitely no security expert, but consider myself educated..
I'm a bit worried about devshed myself.. I've never needed to go there in years, like yourself.. but I feel its still a reasonably high-profile sites for beginners. So it's a bit scary reading that type of stuff.
Evert
Just FYI, escaping strings is not enough. Also validate length for username/passwords. INSERT/UPDATE column truncation behavior can really bite you.