CODEWITHSUNDEEP
c#sqlasp.netsql-server
TheEdge
TheEdge

Reputation: 9881

Protecting data entered SQL against bad behaviour

For some functionality inside our ASP.NET (against SQL Server) app we allow the staff users to specify a WHERE clause. This WHERE clause is then appended in code to a system generated SELECT statement to retrieve some data.

So in essence we end up with:

SELECT SomeFields FROM SomeTable WHERE UserEnteredWhere

where the bold is the system and the italic is the user. This has been working for quite some time and gives us some good flexibility.

However there is clearly a big problem here where if a user enters:

SomeField = 1; DROP TABLE SomeTable;

Currently the only validation is whether the statement starts with a SELECT which will pass because we will end up with:

SELECT SomeFields FROM SomeTable WHERE SomeField = 1; DROP TABLE SomeTable;

The long term solution would be to not allow the user to enter SQL into a text box but in the short term I want to shore this up.

The only solution I can think of is to run each request in a transaction to ensure no long term damage is done.

Has anyone got any suggestions as to how I can validate the user entered "where clause" to ensure that it does not have any DDL content even before it is executed?

PS. I do realise the above is bad and should not be done, but it is what I have in a legacy system (I did not write) so please try and help me with the question instead of telling me don't do that. :-)

Upvotes: 1

Views: 100

Answers (2)

TheEdge
TheEdge

Reputation: 9881

As I needed a quick fix I ultimately ended up tackling this from the SQL Server side of things:

  • By making use of a separate connection in .NET for these database interactions. T
  • The user is locked down to only allow for SELECT statements as follows:

SQL: 

CREATE LOGIN MrReadOnly
    WITH PASSWORD = 'LetMeIn!';

USE MyDatabase;
CREATE USER MrReadOnly FOR LOGIN MrReadOnly;
GO 

EXEC sp_addrolemember N'db_datareader', N'MrReadOnly'

Upvotes: 1

Gordon Linoff
Gordon Linoff

Reputation: 1271111

You know the right answer -- you have exposed your application to SQL injection. In general ad-hoc solutions are not robust. And, putting the statement in a transaction doesn't help. After all, commit could be one of the commands.

Some interfaces have the equivalent of "execute a single query". That would pretty much solve your problem, because a second query would generate an error in the application.

I would suggest not allowing any of the following characters/words in the user-generated conditions:

  • ;
  • update
  • delete
  • insert
  • create
  • exec
  • grant
  • and possibly more
  • and also possibly select (unless you want to support subqueries)

This might limit to some extent what is allowed for the condition.

This is not a guarantee against injection, but something that should make things better.

Upvotes: 3

Related Questions