ACE Team - Security, Performance & Privacy - All Comments

re: Simple Rules To Stop Bad Guys

Daniel — Fri, 18 Dec 2009 01:42:31 GMT

Regarding the SharePoint stuff:

Some of the SQL SharePoint generates against the content db is built on the fly (done properly with parameter substitution). Not any call from the Object model (or COM API) directly calls a sproc. It is mostly simple IF ELSE constructs.

Just take a look using SQL Profiler when provisioning sites or pages to sharepoint sites.

re: Simple Rules To Stop Bad Guys

Daniel — Fri, 18 Dec 2009 01:37:43 GMT

I think anyone here agrees that stored procedures and parameterized queries are always a superior choice, because they offer "some" built-in validation for free(the stored proc still needs application level validation), are easier to maintain and - depends on the perspective - usually perform no worse.

Maybe we are not on the same page regarding the definition of "dynamic" sql: Building the parameterized SQL dynamically and still using @Param-substituted parameters WITH the built-in validation, does that constitute "dynamic" SQL or just "parameterized" SQL? To me, the query is still "dynamic".

But actually, you simply cannot write _any_ database query (even when talking to SQL Server, my favorite relational DB) pre-built (see SharePoint 2007, which uses parameterized, but dynamically built/stitched together queries for some things) or even parameterized. Some of the SQL Server Service Broker statements simply _cannot_ be parameterized when using the ADO.NET interface to call them. You need to use fully "dynamic" SQL here.

So, telling users to "get a new database" is not a good idea:-p SQL Server is still king of the hill.

re: Simple Rules To Stop Bad Guys

TheRockyH — Thu, 17 Dec 2009 12:51:54 GMT

Someone else recently mentioned this to me as well. And after reading it in that light, perhaps you are correct.

However, that sentiment is still incorrect. Securing dynamic SQL is much harder than securing parameterised stored procs.

All things being equal, there are more likely to be mistakes in implementation and filtering with dynamic sql than there are with parameterised stored procs.

So based on our team's 2000+ reviews, and identified bugs, and what we have seen is that dynamic sql is much more prone to error and consequently much more likely to be exploitable. So they are not the same.

Additionally, along the lines of checking facts, SharePoint does not use dynamic sql. There is no dynamic sql created in the web application tier of SharePoint, and all of the database stored procs are parameterised stored procedures. The only time you will have dynamic sql in SharePoint is if you create you own web part and you put it in there. SharePoint itself does not have any.

He may be confused with SPQuery. SPQuery is it's own type of query for finding objects stored in SharePoint. It, like LDAP or XPATH, has its own type of query system. Again it is not Dynamic SQL because it is not SQL against a database. It is actually a thing called Collaborative Application Markup Language (CAML). Bottom line, you can't own an SQL box with SPQuery.

If anyone is interested, here are the links to information about it:

http://blogs.msdn.com/sharepoint/default.aspx

http://msdn.microsoft.com/en-us/library/dd587370(office.11).aspx

And the MS SharePoint team blog has a lot of good info too: http://blogs.msdn.com/sharepoint/default.aspx

re: Simple Rules To Stop Bad Guys

Ronald — Thu, 17 Dec 2009 12:13:09 GMT

I actually understood Daniel's comment as: the attack vector for stored procedures, parameterized queries and sp_executesql is equally big.

re: Simple Rules To Stop Bad Guys

TheRockyH — Thu, 17 Dec 2009 07:22:04 GMT

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.

From: http://cwe.mitre.org/data/definitions/564.html

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.

From: http://msdn.microsoft.com/en-us/library/ms161953.aspx

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",

SqlDbType.VarChar, 11);

Parm.Value = Login.Text;

From: http://unixwiz.net/techtips/sql-injection.html#miti

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. "

-and-

"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)

Mitigations

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."

From: http://www.owasp.org/index.php/SQL_Injection

SQL injection errors occur when:

1.Data enters a program from an untrusted source.

2.The data used to dynamically construct a SQL query

From: http://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet

Primary Defenses

Defense Option 1: Prepared Statements (Parameterized Queries)

Defense Option 2: Stored Procedures

Defense Option 3: Escaping All User Supplied Input

From: http://www.owasp.org/index.php/Reviewing_Code_for_SQL_Injection

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"

re: Simple Rules To Stop Bad Guys

Daniel — Wed, 16 Dec 2009 14:03:39 GMT

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.

re: Simple Rules To Stop Bad Guys

TheRockyH — Wed, 16 Dec 2009 13:53:26 GMT

Sorry Richard, I fixed the link. You will have to login/register on Microsoft Connect, then download it from there. https://connect.microsoft.com/site734/Downloads

re: Simple Rules To Stop Bad Guys

Richard — Wed, 16 Dec 2009 13:16:40 GMT

Where's the missing link to download WACA?

re: Simple Rules To Stop Bad Guys

TheRockyH — Wed, 16 Dec 2009 12:20:10 GMT

Hi Alistair,

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.

re: Simple Rules To Stop Bad Guys

Alastair Upton — Wed, 16 Dec 2009 11:58:33 GMT

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).