Avatar of pcardwell
pcardwell asked on

Looping through form input to sanitize before applying a stored procedure

Please indicate how to correct checking of form input, by using a RegExp that is proposed by jurgenl in the SQL attack question of https://www.experts-exchange.com/Security/Vulnerabilities/Q_23408074.html#a21582240.

The code snippet shows a shortened version of Jurgenl's include ASP, along with an attempt to loop through the form data looking for matches. Below is the ASP including this. At present this is not working at all as we want.

What we want is coding that:-
a) Loops through all the Request.Form parameters, searching for unacceptable text
b) Shows one message (not several) if bad text found in one or more fields
c) Stops the SP proceeding, as we don't want to insert the bad data into the db
THE INCLUDE FILE formfilter.asp
' this creates a global regexp object g_bl for testing strings against sql injection
dim g_bl
set g_bl = New RegExp
g_bl.Pattern = "banner82|xp_|;|--|/\*|<script|</script|ntext|etc"
g_bl.IgnoreCase = true
g_bl.Multiline = true
Dim errormessage
errormessage = "Please enter other input by clicking browser back button"
For Each s in Request.Form
  If g_bl.Test(Request.Form(s)) Then
  Response.Write errormessage  
  End If
<!--#include file="formfilter.asp" -->
Dim Command1__ClientName
Command1__ClientName = NULL
if(Request.Form("ClientName") <> "") then Command1__ClientName = Request.Form("ClientName")
Dim Command1__TitleAgency
Command1__TitleAgency = NULL
if(Request.Form("TitleAgency") <> "") then Command1__TitleAgency = Request.Form("TitleAgency")
Dim Command1__ItineraryLong
Command1__ItineraryLong = NULL
if(Request.Form("ItineraryLong") <> "") then Command1__ItineraryLong = Request.Form("ItineraryLong")
set Command1 = Server.CreateObject("ADODB.Command")
Command1.ActiveConnection = MM_xxx_STRING
Command1.CommandText = "dbo.usp_STORED PROCEDURE"

Open in new window

ASPSecurityVisual Basic.NET

Avatar of undefined
Last Comment

8/22/2022 - Mon
Göran Andersson

The correct way of verifying the input data is to throw away that "formfilter" code, and do it properly instead.

Different kinds of data should be handled in different ways. Code like a "formfilter" can't do that, instead it will stop data that may be perfectly valid, while still not giving a complete protection.

To fully protect yourself against SQL injections, you only have to do two things:

1. Convert strings containing numericals and dates into numerical and date values. A numerical or date value can not contain any harmful code.

2. Encode string values when you put them in the SQL query. This is done differently depending on the database used. Encoding for MS Access and MS SQL Server, for example, is done by doubling any apostrophes in the string.

However, SQL injections is not the only things that you need to protect yourself against. To protect yourself against cross site scripting you should make sure that you don't put anyting in the page that hasn't been properly encoded.

Any data originating from user data should be htlm encoded. Any data that you put in an url should be url encoded. Any data that you put in a Javascript string should be encoded by replacing \ with \\ and the string delimiters (' or ") replaced with \' or \" respectively.

Any data that you have verified as safe is not a problem, of course. A numerical value for example can be displayed in the page without the need for html encoding or url encoding, as it can not contain anything that needs encoding.

Thanks Greenghost, unfortunately we are unsure how to apply this, and not sure it would have worked for us before.

Re 1, we've had 4 SQL injection attacks in a few days, the last 4 days ago. On checking the affected fields in the SQL 2000 database, they were all ntext and nvarchar. The script could not be placed in the int and date columns. These seem well-protected.

Re 2, DW 8 coding seem to do that in most places. The hole was probably one we ourselves coded badly, but are not sure. So we are moving everything to stored procedures. That seems to be the main approach agreed by many against SQL attack. The formfilter is to add additional protection - certainly not to be the only protection.

I do agree that the formfilter may cut out valid data. Right now we want to be very conservative though.

To protect against SQL injection, I have written the attached suite of functions. They are pretty simplistic but essentially allow only digits in numeric fields and make sure that any apostrophes are escaped. They are written for MSSQL so you may have to modify them for other databases.

I agree with GreenGhost, that protecting against handling SQL injection and cross site scripting are not the same. I find that using a white/blacklist approach tends to either allow the potential for new exploits to get round it or for valid input to be rejected. Instead, I make sure that any database content that has been input by a user gets escaped when it is output, using Server.HtmlEncode. This means that any tags appear on the page as text, rather than actual tags.

Another important consideration is database security. If you are using MSSQL, the exploit that is causing problems at the moment can be prevented by denying permission on the sysobjects table to the login used to access the database from the pages.
' Sample usage
Dim sSQL, iNumVar, sTextVar
iNumVar = Request.Form("NumVar")
sTextVar = Request.Form("TextVar")
sSQL = "author_List " & SanitiseInput(iNumVar, "number")
 & ", '" & SanitiseInput(sTextVar, "text")
 & "'" 
' Functions
Function SanitiseInput(sVariable, sFormat)
	Select Case LCase(sFormat)
		Case "number"
			SanitiseInput = SanitiseNumber(sVariable)
		Case "text"
			SanitiseInput = SanitiseText(sVariable)
		Case "date"
			SanitiseInput = SanitiseDate(sVariable)
	End Select
End Function
Function SanitiseNumber(sVariable)
	If Not HasValue(sVariable) Then
		' No value to sanitise, return empty string
		SanitiseNumber = ""
		Exit Function
	End If
	If LCase(Trim(sVariable)) = "NULL" Then
		SanitiseNumber = "NULL"
		Exit Function
	End If
	Dim sChar, i
	For i = 1 To Len(sVariable)
		sChar = Mid(sVariable, i, 1)
		If Asc(sChar) < 48 Or Asc(sChar) > 57 Then
			' Not a digit between 0 and 9
			Exit Function
		End If
		' If we get to here, character is valid, tack onto output
		SanitiseNumber = SanitiseNumber & sChar
End Function
Function SanitiseText(sVariable)
	Dim sOut
	If Not HasValue(sVariable) Then 
		Exit Function ' Nothing to format
	End If
	sOut = sVariable
	sOut = Trim(sOut) ' Knock any spaces off either end
	sOut = Replace(sOut, "'", "''") ' Double up the apostrophes
	SanitiseText = sOut ' Return it
End Function
Function SanitiseDate(sVariable)
	If Not HasValue(sVariable) Then
		' No value to sanitise, return empty string
		SanitiseDate = ""
		Exit Function
	End If
	SanitiseDate = Replace(sVariable, "'", "''")
End Function
Function HasValue(sValue)
	' ** Checks a variable to make sure it is not blank/null
	HasValue = False
	If IsNull(sValue) Then 
		Exit Function
	End If
	If Trim(sValue) = "" Then 
		Exit Function
	End If
	HasValue = True
End Function

Open in new window

Your help has saved me hundreds of hours of internet surfing.

An aside...

..don't be fooled into thinking that simply moving your inline SQL to stored procedures is enough. If you still assemble SQL strings from user input (as above) it does not matter if they are inline SQL or stored procedure calls, they can still be exploited.

The most robust (but also the most time consuming) way to stop all of this is to use an ADO Command object to call your store procedure and pass all input through the parameters collection.

Thanks greengo

I am  looking at your first very helpful comment to see how to apply to our case (SQL 2000). As the SQL injection we suffered was in HEX, in fact it turns out there are only 3 words that could catch it in a blacklist. Very precarious, and probably insufficient for the next attack.

Re your second comment, we are taking the input from the form into the ASP page that calls the SP. The coding produced by Dreamweaver does not seem to assemble SQL strings, just feed the parameters one by one in without further analysis. Below is an extract. Do you think the sample coding below for a few parameters is safe at all? (Note: we have to replace "" by NULL in line2 of each to avoid errors if some input fields are left blank).
Dim Command1__ClientFirstName
Command1__ClientFirstName = NULL
if(Request.Form("ClientFirstName") <> "") then Command1__ClientFirstName = Request.Form("ClientFirstName")
Dim Command1__ClientLastName
Command1__ClientLastName = NULL
if(Request.Form("ClientLastName") <> "") then Command1__ClientLastName = Request.Form("ClientLastName")

Open in new window


Greengo, grateful for comment on how to apply your suite of MSSQL functions, as they look like a great solution.

a) Can all the code be in a SSI?
b) How can we get to loop automatically through the paramters, or must we define each parameter individually?
c) Should this all be in the input form itself, or in the ASP page above that calls the parameters and the SP?
Get an unlimited membership to EE for less than $4 a week.
Unlimited question asking, solutions, articles and more.

You're quite right, it's only a matter of time before the exploit evolves so it makes sense to attack things at the source.

Yes, you can place all those functions in an SSI, include that at the top of your pages and then call it as below. You don't need to do anything about inputs to text columns in the DB as Dreamweaver escapes the apostrophes in those. The ones you need to check/change are the numeric inputs.

From what I remember, Dreamweaver ultimately assembles these into some form of SQL statement. Without seeing the rest of the code, it's difficult to tell but unless it's creating a command object further down and setting parameter objects against it, you need to sanitise the user input in some way. It may be that it creates a command object to execute an SQL statement but this is not enough - it needs to pass inputs through parameter objects in order to be insulated.

As far as automatically looping through the parameters is concerned, I don't belive this is possible because Dreamweaver assembles each one as a separate variable. You need to do it as I've shown below. Trust me, bite the bullet and go through your code carefully even if it takes days. It's worth it because next time the intrusion might not be as easy to clean up. I know of colleagues who've had DROP TABLE commands run against them in this way.

I think all this answers your final question - the code needs to be placed in the section that calls the SP.
Dim Command1__SomeNumericInput
Command1__SomeNumericInput = NULL
if(Request.Form("SomeNumericInput") <> "") then Command1__SomeNumericInput = SanitiseInput(Request.Form("SomeNumericInput"), "number")

Open in new window


Thanks again greengo. We're in the process of applying this, and definitely determined to 'bite the bullet'.

Meanwhile we've bit hit again by another SQL attack, despite removing ALL forms and giving all remaining dynamic pages only read permission. It is a mystery how the exploit was successful.

I have been assuming that we only need to concentrate on sanitising form input. Is this correct? Or do we need to sanitise the SQL of dynamic pages (typically with URL querystrings, eg trip.asp?id=numvar&name=textvar)  even though they only have read permissions?

Greengo, we are applying your suite of functions and had a couple of quick questions:-

a) if we enter any of the attack examples below into a text field, they come out unchanged. Is sanitisation working?
a OR 1=1
a OR 1=1--
" or 1=1--
(pdw =  a); EXEC xp_cmdshell dir C: --
Your email said this sanitisation "make(s) sure that any apostrophes are escaped" but this does not seem to happen.

b) Number sanitisation is working. But I did not understand this part of the function, as it seems impossible to pass "NULL" at all:-
If LCase(Trim(sVariable)) = "NULL" Then SanitiseNumber = "NULL"
This is the best money I have ever spent. I cannot not tell you how many times these folks have saved my bacon. I learn so much from the contributors.

Greengo, with apologies I see that in copying the attack examples in a) above from a webpage, the apostrophe changed into some kind of single quote mark. If replaced by an apostrophe, then the sanitisation doubles this.

Is this sufficient sanitisation to stop an attack though? I entered the actual attack code we suffered recently into both a text and number field. The first text sanitisation left the code completely unchanged, the second cut it down to 10.

Open in new window


Log in or sign up to see answer
Become an EE member today7-DAY FREE TRIAL
Members can start a 7-Day Free trial then enjoy unlimited access to the platform
Sign up - Free for 7 days
Learn why we charge membership fees
We get it - no one likes a content blocker. Take one extra minute and find out why we block content.
See how we're fighting big data
Not exactly the question you had in mind?
Sign up for an EE membership and get your own personalized solution. With an EE membership, you can ask unlimited troubleshooting, research, or opinion questions.
ask a question

Hi greengo, thanks very much for your help and explanation. Now I understand why we were hit again. We spent the whole of Saturday putting another level of protection in place, using your functions and also a simple RegExp. The latter is probably redundant with your functions, but we are happy to be excessive right now!