5

I have a project (private, ASP.net website, password protected with https) where one of the requirements is that the user be able to enter Sql queries that will directly query the database. I need to be able to allow these queries, while preventing them from doing damage to the database itself, and from accessing or updating data that they shouldn't be able to access/update.

I have come up with the following rules for implementation:

  1. Use a db user that only has permission for Select Table/View and Update Table (thus any other commands like drop/alter/truncate/insert/delete will just not run).
  2. Verify that the statement begins with the words "Select" or "Update"
  3. Verify (using Regex) that there are no instances of semi-colons in the statement that are not surrounded by single-quotes, white space and letters. (The thought here is that the only way that they could include a second query would be to end the first with a semi-colon that is not part of an input string).
  4. Verify (using Regex) that the user has permission to access the tables being queried/updated, included in joins, etc. This includes any subqueries. (Part of the way that this will be accomplished is that the user will be using a set of table names that do not actually exist in the database, part of the query parsing will be to substitute in the correct corresponding table names into the query).

Am I missing anything?

The goal is that the users be able to query/update tables to which they have access in any way that they see fit, and to prevent any accidental or malicious attempts to damage the db. (And since a requirement is that the user generate the sql, I have no way to parametrize the query or sanitize it using any built-in tools that I know of).

2
  • This isn't SQL injection if the users are actually writing SQL. It seems the phrase has properly communicated what you meant, though! Commented Jan 14, 2009 at 20:47
  • I thought of that, but I couldn't decide on a better way to phrase it. Maybe "hot to prevent mischievous activity on user generated sql queries"? I am open to changing the title to something that would make more sense... Commented Jan 14, 2009 at 21:04

15 Answers 15

16

This is a bad idea, and not just from an injection-prevention perspective. It's really easy for a user that doesn't know any better to accidentally run a query that will hog all your database resources (memory/cpu), effectively resulting in a denial of service attack.

If you must allow this, it's best to keep a completely separate server for these queries, and use replication to keep it pretty close to an exact mirror of your production system. Of course, that won't work with your UPDATE requirement.

But I want to say again: this just won't work. You can't protect your database if users can run ad hoc queries.

Sign up to request clarification or add additional context in comments.

4 Comments

I agree on this. The only think that I can imagine is to allow just selects on some few tables and nothing else. Even updates can hurt db horribly. And as you mentioned 'bad' selects can kill your db as well.
I've always found it a bad idea to have users at all! Doesn't it depend upon who the users are as to whether this is a bad idea?
No. Even knowledgable users will make a mistake once in a while. You don't want people running ad hoc queries on production data.
Joel, maybe you should restrict your databases so that even you don't have access? At some point someone has to have dangerous access. It is all a matter of judgement and circumstance where you draw the line and how you manage the risk.
4

what about this stuff, just imagine the select is an EXEC

select convert(varchar(50),0x64726F70207461626C652061)

15 Comments

What exactly does this do and how would I convert a query into this? Also, I need to be able to handle Update queries as well.
@Yaakov: run it in a query window and see what the result of the convert is.
For those without a SQL server handy, the above converts to "drop table a". Basically, this is a lesson that you simply CAN'T make a free-text query "safe" just by looking for particular characters or patterns.
This executes a query with "drop table a" showing in the first row/column. How would this be used to execute the "drop table a" statement? Also, if the user only has datareader and datawriter permissions, than I wouldn't have to worry about them making any structural changes to the db, right?
what if it where: exec(select convert(varchar(50),0x64726F70207461626C652061))
|
4

My gut reaction is that you should focus on setting the account privileges and grants as tightly as possible. Look at your RDBMS security documentation thoroughly, there may well be features you are not familiar with that would prove helpful (e.g. Oracle's Virtual Private Database, I believe, may be useful in this kind of scenario).

In particular, your idea to "Verify (using Regex) that the user has permission to access the tables being queried/updated, included in joins, etc." sounds like you would be trying to re-implement security functionality already built into the database.

Comments

3

Well, you already have enough people telling you "dont' do this", so if they aren't able to dissuade you, here are some ideas:

INCLUDE the Good, Don't try to EXCLUDE the bad
(I think the proper terminology is Whitelisting vs Blacklisting ) By that, I mean don't look for evil or invalid stuff to toss out (there are too many ways it could be written or disguised), instead look for valid stuff to include and toss out everything else.

You already mentioned in another comment that you are looking for a list of user-friendly table names, and substituting the actual schema table names. This is what I'm talking about--if you are going to do this, then do it with field names, too.

I'm still leaning toward a graphical UI of some sort, though: select tables to view here, select fields you want to see here, use some drop-downs to build a where clause, etc. A pain, but still probably easier.

2 Comments

I already built that UI, and it was a pain. But it just cant handle complex queries, subqueries, etc.
And the field names kind of handle themselves - there is no restriction on fields they can see within a single table (as long as they can see that table). Update queries though is another issue (see tinyurl.com/8edl3d)
3

What you're missing is the ingenuity of an attacker finding holes in your application.

I can virtually guarantee you that you won't be able to close all the holes if you allow this. There might even be bugs in the database engine you don't know about but they do that allows an SQL statement you deem safe to wreck havoc in your system.

In short: This is a monumentally bad idea!

Comments

2

As the others indicate, letting end-users do this is not a good idea. I suspect the requirement isn't really that the user really needs ad-hoc SQL, but rather a way to get and update data in ways not initially forseen. To allow queries, do as Joel suggests and keep a "read only" database, but use a reporting application such as Microsoft Reporting Services or Data Dynamics Active reports to allow users to design and run ad-hoc reports. Both I believe have ways to present users with a filtered view on "their" data.

For the updates, it is more tricky- I don't know of existing tools to do this. One option may be to design your application so that developers can quickly write plugins to expose new forms for updating data. The plugin would need to expose a UI form, code for checking that the current user can execute it, and code for executing it. Your application would load all plugins and expose the forms that a user has access to.

Comments

2

Event seemingly secure technology like Dynamic LINQ, is not safe from code injection issues and you are talking about providing low-level access.

No matter how hard you sanitize queries and tune permissions, it probably will still be possible to freeze your DB by sending over some CPU-intensive query.

So one of the "protection options" is to show up a message box telling that all queries accessing restricted objects or causing bad side-effects will be logged against user's account and reported to the admins immediately.

Another option - just try to look for a better alternative (i.e. if you really need to process & update data, why not expose API to do this safely?)

Comments

1

One (maybe overkill) option could be use a compiler for a reduced SQL language. Something like using JavaCC with a modified SQL grammar that only allows SELECT statements, then you might receive the query, compile it and if it compiles you can run it.

For C# i know Irony but never used it.

Comments

1

You can do a huge amount of damage with an update statement.

I had a project similar to this, and our solution was to walk the user through a very annoying wizard allowing them to make the choices, but the query itself is constructed behind the scenes by the application code. Very laborious to create, but at least we were in control of the code that finally executed.

1 Comment

Re: damage - there is version control implemented on the affected tables, so any damage made to data can be undone
1

The question is, do you trust your users? If your users have had to log into the system, you are using HTTPS & taken precautions against XSS attacks then SQL Injection is a smaller issue. Running the queries under a restricted account ought to be enough if you trust the legitimate users. For years I've been running MyLittleAdmin on the web and have yet to have a problem.

If you run under a properly restricted SQL Account select convert(varchar(50),0x64726F70207461626C652061) won't get very far and you can defend against resource hogging queries by setting a short timeout on your database requests. People could still do incorrect updates, but then that just comes back to do you trust your users?

You are always taking a managed risk attaching any database to the web, but then that's what backups are for.

3 Comments

The site is password protected, https. The queries will run with a db user that only has select table, select view and update table permissions. There will be version control on the data in the DB itself, so they can always roleback incorrect queries.
You shoudld update your question tags to read asp.net or website so our answers can then be more specific.
I have updated the tags and the first sentence of the question to reflect this.
0

If they don't have to perform really advanced queries you could provide a ui that only allows certain choices, like a drop down list with "update,delete,select" then the next ddl would automatically populate with a list of available tables etc.. similar to query builder in sql management studio.

Then in your server side code you would convert these groups of ui elements into sql statements and use a parametrized query to stop malicious content

2 Comments

I already have a graphical query builder implemented that does what you describe. However, this cannot easily handle the different types of advanced queries that will need to be performed.
I was just going to recommend this. But honestly, if the user is smart enough to construct advanced queries, then they are smart enough to hack your server.
0

This is a terribly bad practice. I would create a handful of stored procedures to handle everything you'd want to do, even the more advanced queries. Present them to the user, let them pick the one they want, and pass your parameters.

The answer above mine is also extremely good.

2 Comments

FYI: The ordering of answers changes based on voting. So referring to "the answer above mine" is not useful in the long term. If you like an answer, upvote it and/or add a comment to it.
Could you provide a link to the answer mentioned?
0

Although I agree with Joel Coehoorn and SQLMenace, some of us do have "requirements". Instead of having them send ad Hoc queries, why not create a visual query builder, like the ones found in the MS sample applications found at asp.net, or try this link.

I am not against the points made by Joel. He is correct. Having users (remember we are talking users here, they could care less about what you want to enforce) throw queries is like an app without a "Business Logic Layer", not to mention the additional questions to be answered when certain results does not match other supporting application results.

2 Comments

There already is a visual query builder. This is to supplement that for advanced queries that cannot be handled visually.
Then, as Joel said, try replication (i'd perfer snapshot), make sure your access permissions are in place and just deploy an off-the-shelf SQL editor, ver basic, instead of re-inventing the wheel.
0

here is another example

the hacker doesn't need to know the real table name, he/she can run undocumented procs like this

sp_msforeachtable 'print ''?''' 

just instead of print it will be drop

2 Comments

but if the db user only has select and update privileges, than they wont be able to execute stored procedures like this, correct?
this proc can be executed by anyone run this EXEC master..sp_helprotect 'sp_msforeachtable' output Owner Object Grantee Grantor ProtectType Action Column dbo sp_MSforeachtable public dbo Grant Execute .
0

Plenty of answers saying that it's a bad idea but somethimes that's what the requirements insist on. There is one gotcha that I haven't spotted mentioned in the "If you have to do it anyway" suggestions though: Make sure that any update statements include a WHERE clause. It's all too easy to run

UPDATE ImportantTable
SET VitalColumn = NULL

and miss out the important

WHERE UserID = @USER_NAME

If an update is required across the whole table then it's easy enough to add

WHERE 1 = 1

Requiring the where clause doesn't stop a malicious user from doing bad things but it should reduce accidental whole table changes.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.