Ways to enforce prepared statements

Question

I just stumbled (by accident) on yet another stupid not-sanitized-at-all sql injection flaw in a project I'm working on ...and I'm so tired of it.
Do you have any advise on how to eliminate such bad sql statements and enforce prepared statements where ever feasible? Right now I would prefer a solution like

REVOKE DarnInlineDataStatements ON * TO xyz

But since this seems unlikely, are there e.g. static code analysis tools for finding these things (to a certain point of reliability)? Or anything else you would recommend?
edit: The soft-skills approach "please don't use them, there are (usually) better ways" didn't seem to work too well in the past. Therefore I would really prefer something that prevents such queries in the first place. Not to deliberately break existing code but for future projects, some "there are no such queries" solution ;-)

Answer 1

You could just search your code base for common T-SQL constructs, such as SELECT, INSERT, UPDATE, DELETE etc.

If you are using Visual Studio 2008 Team System, there is a built-in code analysis rule that will check for some SQL issues. Otherwise, there is the free FxCop.

Answer 2

If you are already using static code analysis tools, you could configure it to look for usage of certain methods, say in Java world Connection.createStatement instead of Connection.prepareStatement.

I think the better approach is to educate the team on ill effects of creating dynamic SQL with concatenation. You must add it to your coding standards document!

Answer 3

Hopefully finding all non-parameterized queries in your software easy enough. Most database access layers like PHP's PDO or Perl's DBI have different calls when preparing query than they do when they just execute SQL. Figuring out what those calls are and searching the code would get you a good start. If your using Linux, grep is your friend.

From there fixing them will be easy enough (I Hope). I don't know the size of the project, but it might behoove you to put queries in a common place or develop some layer based on a common pattern like Active Record / DataMapper / Table Row Gateway.

I don't think there's a good way to automatically enforce it. Perhaps you just need to have a heart to heart with your colleagues. Inline SQL is bad. End of story. If you're the leader, I think a mandate would be in order.

Answer 4

If you move the sql to stored procedures, you can disable SELECT, CREATE, ALTER, UPDATE, DELETE, DROP, etc, permissions for the application user, leaving only EXEC access. That assumes SQL Server, but I'm sure Oracle/MySQL etc allow similar setups.

Note also that this won't guarantee prepared statements: you can still call a stored procedure in an unsafe way. But if you haven't been using a lot of stored procedures it will find any place that was not coded correctly, and if you miss anything it will make it very difficult for an sql injection attack to succeed.

Answer 5

Don't have your application send code directly to the database server, but rather send it thru a middle-tier. It can easily parse whatever's to be sent to the DB and enforce some constraints upon it. For instance, forcing all SQL code to be composed of one and no more than one statement, and either reject it or only execute first statement if there's more than one.

Basic SQL injection attacks involve code like this:

DECLARE @SQL VARCHAR(4000), @Table VARCHAR(50) 
SET @Table='Employees' -- Imagine this actually comes as a parameter from app
SET @SQL='SELECT * FROM '+@Table
EXEC(@SQL)

An attacker could pass string "systables';UPDATE BankAccount SET Money=Money+10000 WHERE AccountCode=12345;DROP TABLE AuditTrails;" to the DB - it'd have disastrous effects.

By having a middle tier doing this minimum parsing on the strings, you are protected against this, the simplest of SQL injection attacks. For others, you can add them to your middle tier (and it can also handle result caching, bandwith throttling, etc.)

If you need to pass more than one SQL statement from your app, I'd say your doing things wrong and should encapsulate that logic in a stored procedure, or minimum at the middle tier itself.