I recently posted an article on our ACE Team blog (you can read it here if you are interested) There were a few comments about the use of Dynamic SQL that made me realise that not everyone “get’s it” yet.
Here are the comments that started the discussion:
Alastair Upton said:
Shouldn’t Rule #4 read ‘Use parameterized SQL?’ Dynamic SQL is perfectly fine (and it is what most ORMs use) provided that it is parameterized. It is not the use of stored procedures that protect you (as you have shown in the example) but the additional protection of parameter validation over string concatenation.
It is also important to protect the various layers of your application (e.g. implementing security on the database tier to make sure minimal rights are granted on objects).
Not really. Dynamic SQL by definition is not fine. It depends on how you are considering ‘parameterized’ SQL. If you are referring to parameterized stored procedures, then yes, that is the way to go but that is not dynamic SQL.
Essentially, dynamic SQL is when you build the SQL up as a string and do all of your variable/parameter substitution (either in code, or in the stored proc itself) before you send the SQL statement to the DB engine to be processed as a command string.
Now, that being said, ORMs, and NHibernate, etc. can produce good SQL, that is technically dynamic, but with proper and extensive input validation.
However these rules are aimed at developers writing code and the code that they write. Not necessarily the libraries they use. Your point is taken though. Thanks for the comment.
Alastair Upton is right, RockyH is wrong. "Abandon Dynamic SQL" is a superficial and wrong statement.
For example SharePoint uses dynamic SQL excessively, and as long as you do it right (and if you do it only when you HAVE to do it, because it also may hurt perf and maintainability), with parameterized queries and optionally sp_executesql, there is no bigger attach vector than with a parameterized sp.
If you write such articles about security, at least get the facts straight.
Now, those of you that know me understand that I couldn’t just let that go… well because I’m concerned about a lost sheep is all. So I’m posting my response to this here, so that we can all make sure we understand that Parameterised Queries on Stored Procedures is THE best way to stop SQL injection attacks and why you should avoid dynamic SQL.
Hello Daniel. Thank you for your passionate response.
So just to be clear you said:
" there is no bigger attach vector than with a parameterized sp." aside from the misspelling of attack I think you are trying to say that Parameterised Stored Procedures are the biggest attack vector. Then you immediately followed with:
"If you write such articles about security, at least get the facts straight."
Excellent advice, especially considering your previous statement.
So just for the sake of getting facts straight, can you give me any examples of where a properly created (not using string concatenation and calling EXEC or sp_execsql which is vulnerable to SQL injection) parameterised stored procedure is exploited through SQL injection?
Here are some of the many many examples that describe where and why you should not use dynamic sql.
Implement SQL strings using prepared statements that bind variables. Prepared statements that do not bind variables can be vulnerable to attack.
Use vigorous white-list style checking on any user input that may be used in a SQL command. Rather than escape meta-characters, it is safest to disallow them entirely. Reason: Later use of data that have been entered in the database may neglect to escape meta-characters before use. Narrowly define the set of safe characters based on the expected value of the parameter in the request.
From: http://cwe.mitre.org/data/definitions/89.html <- This CWE definition is all about not using Dynamic SQL and properly validating input.
Process SQL queries using prepared statements, parameterized queries, or stored procedures. These features should accept parameters or variables and support strong typing. Do not dynamically construct and execute query strings within these features using "exec" or similar functionality, since you may re-introduce the possibility of SQL injection.
Never build Transact-SQL statements directly from user input.
Use stored procedures to validate user input.
Use Type-Safe SQL Parameters
The Parameters collection in SQL Server provides type checking and length validation. If you use the Parameters collection, input is treated as a literal value instead of as executable code
Use Parameterized Input with Stored Procedures
Use the Parameters Collection with Dynamic SQL
If you cannot use stored procedures, you can still use parameters, as shown in the following code example:
SqlDataAdapter myCommand = new SqlDataAdapter(
"SELECT au_lname, au_fname FROM Authors WHERE au_id = @au_id", conn);
SQLParameter parm = myCommand.SelectCommand.Parameters.Add("@au_id",
Parm.Value = Login.Text;
Use stored procedures for database access
When the database server supports them, use stored procedures for performing access on the application’s behalf, which can eliminate SQL [injection] entirely (assuming the stored procedures themselves are written properly).
From: the Microsoft SDL documentation, and quoted on Michael Howard’s blog: http://blogs.msdn.com/sdl/archive/2008/05/15/giving-sql-injection-the-respect-it-deserves.aspx
Applications accessing a database must do so only using parameterized queries. (RH This is a rule at Microsoft and dynamic SQL is considered a high severity bug if found)
Creating dynamic queries using string concatenation potentially allows an attacker to execute an arbitrary query through the application. This vulnerability allows for unauthorized, interactive, logon to a SQL server which may result in the execution of malicious commands leading to the possible modification (or deletion) of Operating System or user data.
From: the SDL documentation: (and Michael Howard’s blog above)
"Applications accessing databases should do so only using stored procedures. "
"Do not use "exec @sql" construct in your stored procedures.
Using stored procedures helps to mitigate the SQL injection threat to a great extent since type checking is available for parameters. If the attacker supplies input that does not match the type constraints the stored procedures will throw an exception. In the vast majority of the cases, this should be properly handled within the application.
However, if the stored procedures perform string manipulation in their code and then execute that query using the "exec @sql" construct incorrect handling of user input can produce the same SQL injection vulnerability as would be seen at the application layer."
From: http://capec.mitre.org/data/definitions/66.html (SQL Injection)
Strong input validation – All user-controllable input must be validated and filtered for illegal characters as well as SQL content. Keywords such as UNION, SELECT or INSERT must be filtered in addition to characters such as a single-quote(‘) or SQL-comments (–) based on the context in which they appear.
Use of parameterized queries or stored procedures – Parameterization causes the input to be restricted to certain domains, such as strings or integers, and any input outside such domains is considered invalid and the query fails. Note that SQL Injection is possible even in the presence of stored procedures if the eventual query is constructed dynamically.
From: http://msdn.microsoft.com/en-us/magazine/cc163917.aspx#S4 Stop SQL Injection Attacks Before They Stop You
"Avoid Dynamic SQL
The SQL injection attacks I have demonstrated in this article are all dependent on the execution of dynamic SQL—that is, SQL statements constructed by the concatenation of SQL with user-entered values. Using parameterized SQL, however, greatly reduces the hacker’s ability to inject SQL into your code."
SQL injection errors occur when:
1.Data enters a program from an untrusted source.
2.The data used to dynamically construct a SQL query
Defense Option 1: Prepared Statements (Parameterized Queries)
Defense Option 2: Stored Procedures
Defense Option 3: Escaping All User Supplied Input
Best Practices when Dealing with Databases
Use Database stored procedures, but even stored procedures can be vulnerable. Use parameterized queries instead of dynamic SQL statements. Data validate all external input: Ensure that all SQL statements recognize user inputs as variables, and that statements are precompiled before the actual inputs are substituted for the variables in Java.
Daniel here are some books I’ve read that you may want to take a look at:
"Writing Secure Code". 2nd Edition. M. Howard and D. LeBlanc. Microsoft. 2003.
"The Database Hacker’s Handbook: Defending Database Servers". David Litchfield, Chris Anley, John Heasman and Bill Grindlay. Wiley. 2005-07-14.
"The Oracle Hacker’s Handbook: Hacking and Defending Oracle". David Litchfield. Wiley. 2007-01-30.
Gray Hat Hacking, Second Edition: The Ethical Hacker’s Handbook by Shon Harris, Allen Harper, Chris Eagle, and Jonathan Ness (Dec 20, 2007)
Chained Exploits: Advanced Hacking Attacks from Start to Finish by Andrew Whitaker, Keatron Evans, and Jack B. Voth (Mar 9, 2009)
The CISSP Prep Guide: Mastering the Ten Domains of Computer Security by Ronald L. Krutz and Russell Dean Vines (Sep 10, 2001)
Hacking: The Art of Exploitation, 2nd Edition by Jon Erickson (Jan 11, 2008)
There are many others but these will help you get started.
Over the past 5 years, I’ve seen a lot of SQL injection vulnerabilities, and properly constructed stored procedures accessed through parameterised queries always fix it. Hopefully we have alleviated your belief that "there is no bigger attach vector than with a parameterized sp"