Compartilhar via

Follow-up from previous SQL Injection post.

  • Artigo

So first off, Thank you to Jim Manico for his comment on my previous post which lead me to create this post. I will includes quotes from Jim’s comment for reference here.

JM: “I think you are terribly wrong, and its important we clear this up.”

No worries I appreciate your candour. Everyone is entitled to their opinion..

I’ll stick to OWASP references since OWASP is near and dear to Jim’s heart, aside from the fact that OWASP is a great reference in general. (Side note, for those of you that do not know Jim, he works for Aspect Security and does the OWASP Podcast Series https://www.owasp.org/index.php/OWASP_Podcast )

According to the OWASP top 10, Injection Flaws ‘particularly SQL injection’ is the #1 most common problem. Why? Because people are using Dynamic SQL. Strongly typed parameterised stored procedures would have solved almost all of these SQL injection attacks. A correctly implemented parameterised stored procedure is not vulnerable to SQL Injection. So if people were using them, SQL Injection wouldn’t be #1 on the IT Security Most Wanted list. But because people keep thinking that it’s ok to use Dynamic SQL (which again is SQL queries based on string concatenation at runtime) it is still one of the most widely used attack vectors for the bad guys.

For those of you just joining us, by Dynamic SQL I am referring to the common industry definition where Dynamic SQL statements are the ones that are created by string concatenation at runtime. For example:

private void cmdLogin_Click(object sender, System.EventArgs e) {

string strCnx =

"server=localhost;database=northwind;uid=sa;pwd=;";

SqlConnection cnx = new SqlConnection(strCnx);

cnx.Open();

//This code is susceptible to SQL injection attacks.

string strQry = "SELECT Count(*) FROM Users WHERE UserName='" +

txtUser.Text + "' AND Password='" + txtPassword.Text + "'";

int intRecs;

SqlCommand cmd = new SqlCommand(strQry, cnx);

intRecs = (int) cmd.ExecuteScalar();

if (intRecs>0) {

FormsAuthentication.RedirectFromLoginPage(txtUser.Text, false);

}

else {

lblMsg.Text = "Login attempt failed.";

}

cnx.Close();

}

From the Data Security Blog

In the previous post/response I listed a plethora of industry references that support avoiding Dynamic SQL.

JM: “Dynamic SQL is important. For example, when you use parametrized queries against tables with VERY [RH large] row size [count] (many millions) against some vendors, its actually KILLS performance. Some features, like advanced search, are often so complex that they beg for dynamic SQL.”

In very rare cases, yes dynamic sql may be unavoidable. Ironically enough, a customer of mine said exactly the same thing. They said that they absolutely had to use Dynamic SQL. And refused to listen when we told them it was a bad idea We said if you really really must use it, you need to do input validation, strongly type the inputs etc. etc. 3 months after we left, they were breached through a SQL Injection vulnerability on a Dynamic SQL Statement that we had told them to fix.

But enough of the first-hand experience stuff…

What does OWASP say you should do to avoid SQL Injection? Here it is:

From: https://www.owasp.org/index.php/Top_10_2007-A2

Avoid the use of interpreters when possible. If you must invoke an interpreter, the key method to avoid injections is the use of safe APIs, such as strongly typed parameterized queries and object relational mapping (ORM) libraries that are immune to injection (be careful here - Hibernate, for example is NOT immune to injection by itself. You have to use named parameters to be safe in Hibernate). These interfaces handle all data escaping, or do not require escaping. Note that while safe interfaces solve the problem, validation is still recommended in order to detect attacks.

Using interpreters is dangerous, so it's worth it to take extra care, such as the following:

  •  Input validation. Use a standard input validation mechanism to validate all input data for length, type, syntax, and business rules before accepting the data to be displayed or stored. Use an "accept known good" validation strategy. Reject invalid input rather than attempting to sanitize potentially hostile data. Do not forget that error messages might also include invalid data
  •  Use strongly typed parameterized query APIs with placeholder substitution markers, even when calling stored procedures
  •  Enforce least privilege when connecting to databases and other backend systems
  • Avoid detailed error messages that are useful to an attacker
  • Show care when using stored procedures since they are generally safe from SQL Injection. However, be careful as they can be injectable (such as via the use of exec() or concatenating arguments within the stored procedure)
  •  Do not use dynamic query interfaces (such as mysql_query() or similar)
  •  Do not use simple escaping functions, such as PHP's addslashes() or character replacement functions like str_replace("'", " "). These are weak and have been successfully exploited by attackers. For PHP, use mysql_real_escape_string() if using MySQL, or preferably use PDO which does not require escaping
  • When using simple escape mechanisms, note that simple escaping functions cannot escape table names! Table names must be legal SQL, and thus are completely unsuitable for user supplied input
  •  Watch out for canonicalization errors. Inputs must be decoded and canonicalized to the application's current internal representation before being validated. Make sure that your application does not decode the same input twice. Such errors could be used to bypass whitelist schemes by introducing dangerous inputs after they have been checked
  • Language specific recommendations:
    • Java EE - use strongly typed PreparedStatement, or ORMs such as Spring or named parameters within Hibernate.
    • .NET - use strongly typed parameterized queries, such as SqlCommand with SqlParameter, or named parameters within Hibernate.
    • PHP - use PDO with strongly typed parameterized queries (using bindParam()).

So just to see if we’re on the same page:

JM: “When you build your queries, you just need to do vendor-specific escaping of your inputs (plus some other validation, like cannonicalization and size checking) while dynamically adding that user data to queries. “

OWASP Says:

“Do not use simple escaping functions, such as PHP's addslashes() or character replacement functions like str_replace("'", ""). These are weak and have been successfully exploited by attackers. For PHP, use mysql_real_escape_string() if using MySQL, or preferably use PDO which does not require escaping”

Now I imagine Jim’s character escaping is perfect but history has shown us that all escaping does for that kind of stuff is give you a false sense of security. Anyone interested in bypassing character escaping should read either of Litchfield’s books on database hacking, or any SQL Injection attacker advice. They both show that it is easy to bypass character escaping.

One of the reasons this fails is because character escaping normally relies on black lists to identify and replace (escape) the characters. Blacklisting by its very nature is very limited and you can never be sure you get every possible bad character into the list.

So now that we have the basics out of the way, let’s get to the crux of the situation. People who are complaining about what I posted are complaining for the wrong reasons. But everyone loves to complain don’t they. ;-)

Had I said, “Avoid Dynamic SQL” instead of “Abandon Dynamic SQL” I wouldn’t have gotten this level of blowback. But that particular word sure stimulated some insightful debate didn’t it?

In reality I agree that there may be some instances where you simply cannot use a parameterised stored procedure to do the right thing. There may be some strange circumstances where you are forced into using Dynamic SQL. I would guess that this is likely to be less than 5% of all database calls a developer will make in a lifetime. But hey, I admit there will be those times.

The problem is, as is evident by the massive number of SQL Injection vulnerabilities reported through the CVE and OWASP demonstrate, very few people get the validation and protection required to use Dynamic SQL right. If Dynamic SQL is so Okay to use, why is SQL Injection public enemy #1 according to OWASP?

The issue is that Dynamic SQL is actually very difficult to secure. It’s the same reason that you don’t use just use strncpy to avoid the problems people have with strcpy. Because people keep getting the ‘n’ part wrong. Which is why so much effort has gone into creating safe string handling functions.

Now personally, I believe that proper input validation would stop almost every attack out there. But perfect input validation is as difficult to achieve as perfect cryptography. And because you can’t implement perfect input validation, you need to apply defence in depth. If you use parameterised stored procedures behind your input validation, even if your input validation fails, you are still immune to SQL injection attacks. Whereas if you are using Dynamic SQL, if your input validation fails you are a victim.

JM: “There are many ways to skin a SQL cat, and we want to make sure that as security professionals we help master techniques that work they way THEY do it, instead of exclusively making them do it our way.”

I agree with you on the fact that there are many ways to skin the SQL Cat.  You can create calls to a database in many different ways. But, you should always use properly created parameterised stored procedures unless there is absolutely no way to avoid it. Then look at the other ways.

As far as what we should do as security professionals; I have my own opinions about this. To be honest, customers pay for us to come in and show them how to secure their applications and development procedures. They are paying us because we have the answers that they need. If all we do is water down our advice until it becomes ineffectual, we are doing the customer an injustice.

As security professionals, if our customers have some bad habits that are leading them to create vulnerable applications, we need to correct what they are doing, not water down the security advice until it allows them to continue to make mistakes.  If there is a point, where the best choice for security simply can not be met, then we find a compromise such as allowing dynamic SQL in rare cases, and we apply our security scrutiny to shoring up the defences around it. But if we are serious about securing modern software, we need to consider changing the way we do things so that the best security choice IS possible rather than bending all the rules to continue to allow the bad habits to propagate.

Consider this; if you do have one of those complex or convoluted SQL Queries that you can’t convert to parameterise stored procedures, then maybe you should re-evaluate how your data is organised, and how you are retrieving the values.  That might address the problem, rather than just the symptom which is the complex dynamic sql.

Again, I will reiterate that this is my opinion. I’m in the security field to change the industry so that we don’t have vulnerable software anymore. Sure in all practicality it’s a pipe dream, but if you aim for the stars even if you fall short you still made it to the moon and that is far better than where we are now.

Now, Jim I hope you did not take this personally as it was not meant that way. But you didn’t provide any facts, evidence, or references for me to address so I responded to the issues as they were presented. I always value comments especially from opinions differing from mine because I believe that the debate itself can be as enlightening as any form of instruction and I’m always eager to learn just in case I have been mis-informed. 

 {Edited for Typos by RH}

Comments

  • Anonymous
    December 21, 2009
    The comment has been removed

  • Anonymous
    December 21, 2009
    Hi Jim, I edited the post for typos (although i did not publish the other comment you made that essentially just contained the typo corrections. By all means we need to supply guidance around how to secure dynamic SQL when it is the absolute only way to do things. Just make sure that some of that guidance includes trying to make it so that parameterised stored procs become feasible. Then we move into Machiavellian input validation around Dynamic SQL)

  • Anonymous
    June 25, 2010
    The comment has been removed