5

i am creating a postgreSQL database reader which also includes a method for a user to type their own query. i want to protect the database by checking if the typed query contains any modifying code. this is my check:

    private bool chech_unwanted_text(string query)
    {
        if (query.Contains("DELETE") || query.Contains("delete") || query.Contains("CREATE") || 
          query.Contains("create") || query.Contains("COPY") || query.Contains("copy") || 
          query.Contains("INSERT") || query.Contains("insert") || query.Contains("DROP") || 
          query.Contains("drop") || query.Contains("UPDATE") || query.Contains("update") || 
          query.Contains("ALTER") || query.Contains("alter"))
        {
            return false;
        }
        else return true;
    }

is this the right method to check for a edit-safe query or is there an other, more reliable way to achieve this?

i know about granting rights to users but that is not working because i don't have a super-user account.

Moonlight
  • 708
  • 1
  • 7
  • 18

5 Answers5

9

You should handle this by using an account with only read access the the database, not by checking the query. Most DBMSs have a privilege mechanism to handle this kind of thing, and PostgreSQL certainly does.

Thomas Levesque
  • 286,951
  • 70
  • 623
  • 758
1

Granting users to type their own queries in never a good idea. The only safe way to do it is to grant them read-only rights (which you said you are unable to do).

Regarding your code snippet, a better way to check if you query contains a verb is using LINQ:

private readonly string[] verbs = new string[]
   { "delete", "create", "insert", ... };

private bool check_unwanted_text(string query)
{
    // convert to lowercase
    query = query.ToLowerInvariant();

    // can any verb be found in query?
    return verbs.Any(v => query.Contains(v));
}

But no, I would not consider using this for SQL sanitizing.

vgru
  • 49,838
  • 16
  • 120
  • 201
1

There is no reliable, general way of checking whether given query is changing database contents, basing only on query text.

Consider this query:

SELECT * FROM myview;

Where myview is defined as follows:

CREATE VIEW myview AS select foo, bar FROM myfunc();
filiprem
  • 6,721
  • 1
  • 29
  • 42
1

If you can't make the account read-only, you still can make a particular transaction read-only by issuing a SET TRANSACTION READ ONLY SQL command immediately after the beginning of the transaction. Then any attempt to modify data in this transaction would fail with an

ERROR: transaction is read only

Daniel Vérité
  • 58,074
  • 15
  • 129
  • 156
  • although its not fail-save, (users can put -- behind their query) its a nice alternative. (i am not using this because i got my read-only account after some messing around) – Moonlight Jan 10 '12 at 07:41
0

Look at this for a little shorter comparsion: Case insensitive 'Contains(string)'

Basically make the comparsion case insensitive (otherwise you would need a lot of code to check for different versions of DeLetE (DELEte etc.)

Also, you could build a list with forbidden keywords and loop through that, maybe a little cleaner?

What will happen if the user needs to query something with these keywords?

SELECT IsDeleted FROM Users;

Maybe you can look at allowing parameters or perhaps build your own "query-builder" depending on the complexity of querys (a bunch of dropdowns based on whats in the database?).

Just a few suggestions.

But you should, if in any way possible, follow the suggestion about read-only user.

Community
  • 1
  • 1
Sverker84
  • 465
  • 3
  • 8
  • i alreaddy have an own build query creator that can handle 90 percent of all the things you might want, only for some rare thing i wanted to implement an 'insertable' query. the way of using case-insnesative is a way i will check when the read-only user is not working/not getting – Moonlight Jan 09 '12 at 10:48
  • The post about using linq seems like a good choice for handling this issue + perhaps some form of parameterized query if at all possible (but then I guess you wouldn't be needing the ability to write custom queries in the first place). – Sverker84 Jan 09 '12 at 10:56