November 8, 2005 at 8:11 am
Hi all
I'm rewriting a stored procedure that validates a string (checking for sql injection) Currently it's a mess of while loops, cursors (over a table of prohibited words) and UPDATETEXT statements, and it runs exceedingly slowly.
There's a list of prohibited keywords. The string been validated may not contain any of the keywords, unless they are within a quoted section. The string may not contain ; or -- and may not have a mismatched number of quotes. The length of the string is variable and may exceed 8000 characters
Anyone have any suggestions as to how to go about this?
CREATE TABLE KeyWords (
Word VARCHAR(20)
)
INSERT INTO KeyWords VALUES ('Delete')
INSERT INTO KeyWords VALUES ('Truncate')
INSERT INTO KeyWords VALUES ('Drop')
DECLARE @ValidString1 VARCHAR(200), @ValidString2 VARCHAR(200), @InvalidString1 VARCHAR(200), @InvalidString2 VARCHAR(200)
SET @ValidString1 = 'This is a valid string. There are no prohibited words or characters.'
SET @ValidString2 = 'This is also valid. The prohibited words are within quotes ''Please drop;'' '
SET @InvalidString1 = 'This is invalid; Bad character and mismatched quotes '' '
SET @InvalidString2 = 'This is also invalid. It is a request to delete the database and truncate tables.'
Thanks in advance
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
November 8, 2005 at 3:06 pm
First off, there's probably a few ways already addressed on this site of how to limit injection attacks more simply. Do a search for "injection".
Secondly, if you have your application use a SQL login with more limited permissions, you don't need to worry if they try to do a truncate, delete, drop, etc.
Thirdly, why do you rush to use a cursor....? Anytime you find yourself writing a DECLARE CURSOR statement, stop and think: "There's got to be a way...." Trust me, there always is a better way.
But on the simple concept of word validation...
You could use "REPLACE" on the string to convert all whitespace to some uncommon bracketing character like "|"
then just join on it
like
Set @StringToBeSearched = REPLACE( @@StringToBeSearched, ' ', '|')
Set @StringToBeSearched = REPLACE( @@StringToBeSearched, CHAR(13), '|')
Set @StringToBeSearched = REPLACE( @@StringToBeSearched, CHAR(10), '|')
(all these REPLACEs could be embedded in one statement)
IF EXISTS ( SELECT 1 FROM RestrictedWordTable
WHERE CHARINDEX( RestrictedWordColumn, @stringtobesearched) > 0 ) THEN error
Or join the index of that word/condition to an indexerrornumber
November 8, 2005 at 3:07 pm
how'd that double @ get in there?
November 8, 2005 at 6:04 pm
If you never drop temporary tables within SP?
And if yes are you sure you'll never do?
Same question about "Delete" and "Truncate".
And are you sure "Drop table" is more dangerous than "Select * from Table"?
You can restore corrupted database but never fix leak of confidential data.
If you really need dynamic SQL generate it inside SQL Server, never allow to pass statements from application.
_____________
Code for TallyGenerator
November 8, 2005 at 11:39 pm
OK, a few things that maybe weren't clear.
I'm rewriting this proc because it's a performance hog. I didn't write the code in the first place. I hate cursors.
The app's a custom reporting app (high-level description) and users are allowed to create dynamic filters which are passed to the database and added to the where clause of the sql statement. The where clause is what is checked. The where clause that the front-end generates is quite simple, so select is as prohibited as drop. The word list I gave was just a small subset.
The front-end does check for injection, this is basically last-chance paranoia at work.
I hope that clears things up.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
November 9, 2005 at 1:37 pm
one more thing on the delimited searching using charindex:
wrap the compare words in delimiters:
restricted word list Err_no
|DROP| 1
|DELETE| 2
...
and once you've done the replace on the search string it might look like this:
"cust_no = 123456 --
DROP DATABASE customer"
"|cust_no|=|123456||||DROP|DATABASE|customer|"
Remember to convert comment marks "--;/*;*/" to delimiters as well. Note space, char(13), char(10), and -- converted to delimiters.
November 9, 2005 at 2:04 pm
Do users write SQL script from screen?
I don't think so.
So, there is a set of possible parts of SQL statements recorded in the application code. Application just picks some of them according to controls selected on screen.
What you need to do is to copy those strings to SP code and produce SQL statements inside SP according to set of parameters are reflecting state of controls.
This way you have set of allowed strings, but not restricted ones.
If you forget to allow something you can add it later, if you forget to restrict - it's probably too late to fix at the time you discovered such mistake.
_____________
Code for TallyGenerator
November 9, 2005 at 11:02 pm
No, but users do enter values.
eg. User selects to filter on company code and lists companies 'ABC123','DEF789','XYZ000'
or, he selects company name and would like to do a wildcard match for all companies whose name starts with 'Ned'
I got some good ideas from the other articles and posts around, so thanks.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
Viewing 8 posts - 1 through 7 (of 7 total)
You must be logged in to reply to this topic. Login to reply