SQL Injection attack code being too intrusive

GravyMonster · 10 Jan 2009 at 15:47

Hey all,

A while back someone posted a regexp function which would check for the usual elements of a SQL query to make sure no injections were happening - all good. The only problem is I've got this on a site now and it's causing a few issues.

For example, the phrase 'Daddy or chips?' is being flagged as an injection attempt.

I've looked through the expression and can't see exactly where it's coming from, could any of you kindly souls help me out?

Code:

	szPattern = "SELECT((.|\s)*?)FROM((.|\s)*?)$|" & _
		"UPDATE((.|\s)*?)SET((.|\s)*?)$|" & _
		"INSERT[\s]+INTO((.|\s)*?)$|" & _
		"DELETE[\s]+FROM((.|\s)*?)$|" & _
		"(DROP|CREATE|ALTER|TRUNCATE)[\s]+TABLE[\s]+((.|\s)*?)$|" & _
		"UNION[\s]+(ALL|SELECT){1}[\s]+((.|\s)*?)$|" & _
		"DECLARE((.|\s)*?)[\s]+(NVARCHAR|VARCHAR|CHAR){1}((.|\s)*?)$|" & _
		"AND[\s]+((.|\s)*?)CONVERT((.|\s)*?)SP_PASSWORD$|" & _
		"[\r?\n|\r|\x00|\x1a]|[-]{2}"

Ta muchly!

GravyMonster · 10 Jan 2009 at 18:51

After some testing I've discovered it's catches whenever there is a ? in the string - can't see where in the pattern that's hits though :confused:

philjohn · 10 Jan 2009 at 21:02

why exactly do you need to do this, are any parts of the query entered by the user (apart from data to be inserted/updated/queried ?). If not then simply using proper escaping and/or prepared statements is all you need.

Inquisitor · 10 Jan 2009 at 21:05

This is bad bad bad.

Either escape your input or, preferably, use prepared statements, as philjon said.

Also, why are you using Systems Hungarian notation? Especially in VB :confused:

suarve · 11 Jan 2009 at 09:27

Inquisitor said:
This is bad bad bad.

Either escape your input or, preferably, use prepared statements, as philjon said.

Also, why are you using Systems Hungarian notation? Especially in VB

Inquisitor · 11 Jan 2009 at 10:04

http://en.wikipedia.org/wiki/Hungarian_notation#Systems_vs._Apps_Hungarian

GravyMonster · 11 Jan 2009 at 11:46

Firstly the user input is being escaped. The reason this is being implemented is because the client wants to know if anyone is trying to subvert the system and if so, who, when and what they tried.

Secondly I'm using systems notation because it's what i learnt using and old habits die hard

So back on topic, anyone got any ideas? I was hoping someone would be able to spot it straight away without me having to actually do some work and dissect the regexp

Shoseki · 11 Jan 2009 at 12:54

Just use PreparedStatements and don't bother with all this stuff?

Inquisitor · 11 Jan 2009 at 13:00

Spunkey said:
Firstly the user input is being escaped. The reason this is being implemented is because the client wants to know if anyone is trying to subvert the system and if so, who, when and what they tried.

…

So back on topic, anyone got any ideas? I was hoping someone would be able to spot it straight away without me having to actually do some work and dissect the regexp

You're never going to get any method that's 100% reliable I'm afraid – it's an impossibility.

Edit: To illustrate the complexity of the problem, here's a paper on it (albeit specific to Oracle):

http://www.securityfocus.com/infocus/1714

Specifically:

The short answer is definitely yes... err... well err... probably... that is, yes it is possible to detect SQL injection but probably not all of the time for all cases and not always in real time. The reasons for this are many and complicated:

There are many different forms of SQL injection attacks that can take place - these are limited only by the hacker's imagination and the DBA's foresight (or lack thereof) to protect the database and provide the least privileges necessary.

Identifying SQL that shouldn't be there is not simple. The reason SQL injection is possible is because of the use of dynamic SQL in applications. This intended dynamic SQL means that the set of all legal SQL statements is harder if impossible to define. If the legal statements are impossible to define then so are the illegal ones.

Distinguishing normal administration from an attacker is not always easy as an attacker can steal an administrator's account.

Detecting SQL injection inevitably involves parsing the SQL statement for possible additions or truncations to it. Table names and view names need to be extracted and checked to see if they should be altered.

For a technique to be useful it should not affect the performance of the database too much.

Corroborating data such as usernames and timestamps are also need to be extracted at the same time.

Many more...

Spunkey said:
Secondly I'm using systems notation because it's what i learnt using and old habits die hard

You should give this one the boot I think, especially as strings are not null-terminated in either VB or VB.NET