dealing with sql injection

[esthera]

i use on my page
that takes in catid form the request("id")

if isnumeric(catid) then
catid=cdbl(catid)
else
catid=0
end if
if catid="" or catid=0 then response.redirect "default.asp"

for some reason the page was still hacked by calling
category.asp id=72;DECLARE%20@S%20VARCHAR(4000);SET%20@S=CAST(0x4445434C415245204054205641524

How could they do this? wouldn't the catid be set to 0 and then nothing happen and just redirect to the home page?

[binaryWeapon]

No, the catid IS a number. [...]id=72[...] They were able to insert malicious code after it though. I can't give you the specifics of how they did it, as I have no knowledge of ASP, but they were able to declare ID as a number, and then add codes after it, which wasn't checked by the if.

[esthera]

but how could it have let the whole string in if I converted it to an integer?

[Spudhead]

Ok, this is confusing me too but there's a few possibilities, and some things missing from your post.

First, post the exact, complete code from where you initially get the id variable (and you should be using request.form() or request.querystring(), not just request()) up until you build and execute the SQL.

Second, what database are you using? MySQL? MSSQL?

Third, that doesn't look like the complete string that they passed in to me - the question mark is missing for a start (assuming this was passed in via a querystring) and it seems to end a bit sudden. Again, post the exact, complete string that your application received.

Ideas-wise... yes, converting it to a double (not an integer - you're doing cdbl(), not cint()) should kill anything non-numberish - or at least throw some errors. But there are some weird things you can do with character encoding and I'm wondering if they've managed to somehow stuff a single quote in there or something. Again, it's difficult to see without seeing the exact code.

Incidentally, have you tried simply altering the page so it response.writes() the generated SQL string instead of trying to execute it, and then passing in the string you think hacked it?

[esthera]

i'm doing request.querystring

db is mssql

i don't understand how cint wouldn't have truncated all that

[brazenskies]

becuase only numbers would be valid in the string!

[esthera]

and it will still pass the isnumeric test -even though there are numbers and text?

[binaryWeapon]

Again, I know nothing about ASP, so just throwing out random ideas here :p

Doesn't the semicolon (;) after the id=72 basically end the declaration of the id variable and then allow for the malicious code to be inserted?

[Spudhead]

Post your code. And post the full querystring. With what you've posted so far - no, I can't see how it would work, either. isNumeric() will return false for characters it believes don't represent numeric values. But since you say it DID work, then I can only assume that there's something we haven't seen yet.

[Spudhead]

Doesn't the semicolon (;) after the id=72 basically end the declaration of the id variable and then allow for the malicious code to be inserted?

It can do, although I think it depends on server config. And in any case, if it marked the end of the querystring variable, anything after it would be discarded before it even reached the point of isNumeric validation. Err. I think.

[Basscyst]

Something you can do to prevent this type of injection is to enclose all user defined data represented in your SQL string in single quotes. Even integers. This prevents extra code from being inserted into the end of the statement.

So your SQL would look something like this:

"SELECT * FROM SomeTable WHERE RecordID = '" & RecordID & "'"


If you just do this:

"SELECT * FROM SomeTable WHERE RecordID = " & RecordID


You've now left it open for injection.

Say your record ID was compromised as in your case above, and suppose record id now held the value of:

72;DELETE FROM TableName

Your sql statement would now look like this:


"SELECT * FROM SomeTable WHERE RecordID = 72;DELETE FROM TableName"


Which would be bad, and is likely similar to what happened to you.

By enclosing the integer data in quotes, the same statement would look like this:


"SELECT * FROM SomeTable WHERE RecordID = '72;DELETE FROM TableName'"


Which would result in an error if your target column's data type is an integer or just select nothing if it is a var char.

You should also replace any single quotes from the user received data prior to the sql execution. This is just good habit, don't ever skip it even if you think you've prevented it in some other manner.

Function SQLSafe(Str)
SQLSafe=Replace(Str,"'","''")
End Function


So in the end a fairly safe version of this would look like this:


"SELECT * FROM SomeTable WHERE RecordID = '" & SQLSafe(RecordID) & "'"


Hope that helps,
Basscyst