views:

48

answers:

3

I'm building my own clone of http://statoverflow.com/sandbox (using the free controls provided to 10K users from Telerik). I have a proof of concept available I can use locally, but before I open it up to others I need to lock it down some more. Currently I run everything through a stored procedure that looks something like this:

CREATE PROCEDURE WebQuery 
    @QueryText nvarchar(1000)
AS
BEGIN
    -- no writes, so no need to lock on select
    SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED 

    -- throttles
    SET ROWCOUNT 500 
    SET QUERY_GOVERNOR_COST_LIMIT 500

    exec (@QueryText)

END

I need to do two things yet:

  1. Replace QUERY_GOVERNOR_COST_LIMIT with an actual rather than estimated timeout, so no query runs longer than say 2 minutes.
  2. Right now nothing stops users from just putting their own 'SET ROWCOUNT 50000;' in front of their query text to override my restriction, so I need to somehow limit the queries to a single statement or (preferrably) disallow the SET commands inside the exec function.

Any ideas?

A: 

On a very basic level, how about blocking any statement that doesn't start with SELECT? Or will other query starts be supported, like CTE's or DECLARE statements? 1000 chars isn't too much room to play with, but i'm not too clear what this is in the first place.

UPDATED

Ok, how about prefixing whatever they submit with SELECT TOP 500 FROM (

and appending a ). If they try to do multiple statements it'll throw an error you can catch. And to prevent denial of service, replace their starting SELECT with another SELECT TOP 500.

Doesn't help if they've appended an ORDER BY to something returning a million rows, though.

CodeByMoonlight
It's a good thought, but you could just query `'select 1;set rowcount 50000;select * from posts;'`
Joel Coehoorn
I knew it was too simple to work *doh*
CodeByMoonlight
It does sort of work- the page as is would only show the '1' result set and not the large result set, but it would still allow people to create a denial of service situation.
Joel Coehoorn
Updated to another possibility now
CodeByMoonlight
No good. Then it's just `'SELECT 1) t; SELECT * FROM ( <My query> '`
Joel Coehoorn
Then some additional parsing :1) check for apostrophe positions. Characters or words identified thus as in a string are ok (i hope). Outside those, check for your danger words and symbols like ; or SET. Remove any carriage returns from the input and replace them with spaces to negate the possibility of -- commenting out half a line
CodeByMoonlight
2) outside the 'safe' strings, check the counts of apostrophes and position. If the number of ( and ) is different, or at any point reading left-to-right there are more ) than (, the query is invalid.
CodeByMoonlight
I'm sure i can come up with more.
CodeByMoonlight
A: 

Throwing this out there:

The EXEC statement appears to support impersonation. See http://msdn.microsoft.com/en-us/library/ms188332.aspx. Perhaps you can impersonate a limited user. I am looking into the availability of limitations that may prevent SET statements and the like.

David Andres
+3  A: 

You really plan to allow users to run arbitrary Ad-Hoc SQL? Only then can a user place in a SET to override your restrictions. If that's the case, you're best bet is to do some basic parsing using lexx/yacc or flex/bison (or your favorite CLR language tree parser) and detect invalid SET statements. Are you going to allow SET @variable=value though, which syntactically is a SET...

If you impersonate low privileged users via EXECUTE AS make sure you create an irreversible impersonation context, so the user does not simply execute REVERT and regain all the privileges :) You also must really understand the implications of database impersonation, make sure you read Extending Database Impersonation by Using EXECUTE AS.

Another thing to consider is deffering execution of requests to a queue. Since queue readers can be calibrated via MAX_QUEUE_READERS, you get a very cheap throttling. See Asynchronous procedure execution for a related article how to use queues to execute batches. This mechanism is different from resource governance, but I've seen it used to more effect that the governor itself.

Remus Rusanu