1

Is there a good way to parse a SQL statement to ensure it contains only a SELECT statement and nothing else? I am using C#, System.Data.SqlConnection, and MS SQL Server

Rachel
  • 130,264
  • 66
  • 304
  • 490

5 Answers5

6

You should connect to the database as a user that doesn't have permission to do anything other than a SELECT.

This way, any non-SELECT statement will be unable to execute.

This is the most secure solution possible, short of duplicating SQL Server's parser.

SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
  • Would I be able to create a user on SQL Server 2005/2008 that can run stored procedures in addition to any SELECT command? – Rachel Nov 09 '10 at 16:21
  • Almost +1.. This is probably the most secure way however it does not answer the question directly.. – Mike Dinescu Nov 09 '10 at 16:23
  • I'd say this is an "X/Y question"... and this answer solves the X. – GendoIkari Nov 09 '10 at 16:24
  • 1
    @Rachel - be careful though that a user who has EXECUTE rights on stored procedures may be able to modify data if he can execute a stored procedure which modifies data - even though the user doesn't have any other privileges than SELECT on tables.. – Mike Dinescu Nov 09 '10 at 16:25
  • @Miky, the direct answer to this question is a solution that is just too big, complicated, heavy and unsure to actually be considered. – Alex Nov 09 '10 at 16:26
  • You should be able to set permissions on each stored procedure though. So just don't give permissions for any stored procedure that does anything other than SELECT. Unless you want them to be able to use stored procs to do other things. – GendoIkari Nov 09 '10 at 16:26
  • @Miky: Yes. Obviously, you should only grant permission for sprocs that you want the user to be able to execute. – SLaks Nov 09 '10 at 16:26
  • This actually answers my question exactly, thank you :) Most of the application updates data via stored procedures (even a Delete sp only sets an IsDeleted flag in most cases, it doesn't actually delete the record) and my biggest concern is I didn't want anyone running DROP/TRUNCATE/ALTER etc commands on the database through the WCF service. – Rachel Nov 09 '10 at 16:40
1

I guess you can come up with a regular expression, but it is likely that it won't be 100% safe.

The best way to do this is to:
1. Either write stored procedures and views, and limit the user's rights to using only them. (and SELECT statements on certain tables)
2. Build a Data Abstraction Layer. You build the queries, not someone else. Let the others access only some of your methods you expose.
3. Use LINQ to SQL, but conceal the DataContext object, so no changes to the database could be made.

Alex
  • 14,338
  • 5
  • 41
  • 59
  • The sql statement needs to be dynamic so sprocs are out. It is for a QueryBuilder engine which allows users to select almost anything from the database. I would also prefer to avoid LinqToSql since the admin users are the ones that define what can be selected from the database, not me. – Rachel Nov 09 '10 at 16:43
0

I think parsing the sql will be the solution: from another question

Community
  • 1
  • 1
Kell
  • 3,252
  • 20
  • 19
  • Even with ANTLR, this will still be very difficult and extremely easy to get wrong. (You need a complete and accurate grammar) – SLaks Nov 09 '10 at 16:35
  • Agreed, but it does beat searching for the word "SELECT" in the query string. I wonder, if you are using a SQLCommand object, can you check the CommandType property before execution? – Kell Nov 09 '10 at 16:40
  • No. CommandType allows you to specify whether the text is a query or sproc. It's never set automatically. – SLaks Nov 09 '10 at 16:43
  • What a pity. You would think with the whole SQL server namespace being available you would be able to find a parser in there somehwere! – Kell Nov 09 '10 at 16:52
-1

Since a SELECT statement would need to be at the very beginning of the statement, you can just check the string to see if the first 6 characters are SELECT:

if (stringSql.Substring(0, 6).ToUpper() == "SELECT")
{
    //execute statement
}
GendoIkari
  • 11,734
  • 6
  • 62
  • 104
  • 1
    This would fail. I could write SELECT * FROM Users; DELETE FROM Users; – Justin Niessner Nov 09 '10 at 16:16
  • If the query uses a CTE, it would start with WITH, not SELECT – Pondlife Nov 09 '10 at 16:17
  • Good point... but Justin is right; you should be able to use parameters to protect yourself. – GendoIkari Nov 09 '10 at 16:18
  • I can't use parameters because the statement needs to be dynamic – Rachel Nov 09 '10 at 16:18
  • @JustinNiessner what if we checked that the statement doesn't include any "DELETE , INSERT , UPDATE , DROP" calls ? is that enough to mark it as read request ? of course it won't be 100% accurate but i believe it's tolerated in many scenarios. – Mawardy Jul 29 '20 at 11:56
-1

Check to make sure the statement begins with a SELECT keyword, and then make sure the statement contains no semicolons (which would begin another SQL statement) that are not parts of literal strings.

Gabriel Magana
  • 4,338
  • 24
  • 23
  • SQL Server doesn't require the semi-colon to separate statements – Rachel Nov 09 '10 at 16:20
  • Checking for a "semicolon that is not part of literal strings" means to analyze the syntax, and as some comments to other answers stated, a select statement might start with a "WITH"... – chiccodoro Nov 09 '10 at 16:20
  • @Rachel really? It's part of the SQL standard to separate statements with semicolons. Do not fonsude GUIs that let you use a different terminator with the TSQL language. – Gabriel Magana Nov 09 '10 at 16:33
  • @chiccodoro: Yes it means to analyze the syntax. How can you know what kind of statement there is if you do not analyze the syntax? – Gabriel Magana Nov 09 '10 at 16:34
  • @gmagana - I think so... I can write `select 1 select 1` on a single line in a new query window and it executes it as two separate commands – Rachel Nov 09 '10 at 16:51
  • @gmagana, my point is: Using a syntax is quite a complex thing. If Rachel really has to analyze it, she will probably use a given lexer, so she can just as well "ask" the lexer whether or not the parsed statement is a select statement instead of relying on the "SELECT" keyword being at position 0 of the string. (Regular expressions won't suffice to check for a semicolon which is not part of a literal string.) – chiccodoro Nov 10 '10 at 07:30