Is this good enough asp/sql input sanitation for a company intranet?

Factual Questions
1

Our company intranet ASP pages use the following function to sanatize input.

I haven’t learnt any proper sanitation techniques (such as parametrized input) but I do make sure to use this function (it wasn’t written by me. It was written by the people from whom I took over responsibility of all the ASP code)

Is it adequate for a company intranet (full of exactly three people who know how to inject sql none of whome are likely to do it :smiley: )



Function Validate_Input(strInput)

	Dim Good_Chars, Bad_Words, I, c

	Good_Chars = "1234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-?"
	Bad_Words = array("select","insert","update","delete","drop","--","'")
	Validate_Input = True

	For I = 1 to len(strInput)

		c=mid(strInput,I,1)
		If (instr(Good_Chars,c)=0) Then
			Validate_Input = False
			Exit Function
		End If

	Next

	For I = lbound(Bad_Words) to ubound(Bad_Words)

		If (instr(1,strInput,Bad_Words(I),vbtextcompare) <> 0) Then
			Validate_Input = False
			Exit Function
		End If

	Next	

End Function
2

While this might protect you, it is fairly lousy code, mostly because it disallows a lot of potentially safe data. For example, it would suck if your surname had the syllable “drop” in it, or any spaces (my surname is of Dutch origin and is in the form “van Surname”, and is constantly mangled by badly written code - this code would simply disallow the correct spelling of my name).

I don’t know if you are using it for names or not, but it sounds like it is being used as the generic sanitizer for everything, which in my opinion makes it bad practice as it is currently written.

I would recommend something like parametrized input. After all, there may not be anything wrong with storing the comment “Hey, make sure you don’t run DROP TABLE all_the_important_data” from a form, you just want to ensure that it cannot inadvertently be run.

3

Exactly. You want to use whatever mechanisms that are available in the SQL API to automatically quote all input. If your SQL API does not provide something like that for at least “standard” queries plus a known-to-be-correct quote() function, drop it and find one that does.

Input validation is useful to check that the input confirms to a known pattern (and to check that required fields are filled in), but in practice, for almost everything but numbers and computer-generated codes, the patterns boil down to “at least one random character”. If you’re using input validation to prevent SQL injection you’re doing it wrong.

4

No, it’s not good enough. You must use your API’s parameter substitution or quoting function to do this.

If you don’t believe me, read my first sentence again in the voice of that school teacher you had when you were little. You know, the one you were really scared of.