SQL Injection Prevention Quickfix.. will it work

[Michael Kujawa]

Hi All,
I have been given a site to redo. In the process of looking at the code,
the live site is open to SQL injection. I know what needs to be done but
limited time right now to redo correctly. In the interm while I am rewriting
the site, will adding a few lines of code as below prevent SQL injection
until I have the time to rebuild the functions and move to stored
procedures.

Basically client side I added a onKeypress javascript routine
to look for ' or " and disallow in login fields

<script>
function checkcode()
{
if(event.keyCode==34 || event.keyCode==39){event.returnValue = false;}
}
</script>

ServerSide I then added an if else statement to trap if user has javascript
disabled

<%
if request.form("submit")="Login" then
if len(rtrim(request("UserID")))>0 and len(rtrim(request("Password")))>0
then
' line added to trap single - dbl quote
if (instr(rtrim(request("UserID")),"'")=0 or
instr(rtrim(request("password")),"'")=0) and
(instr(rtrim(request("UserID")),"""")=0 or
instr(rtrim(request("password")),"""")=0) then
rs.open "select * from FTSUsers where UserID='" &
rtrim(request("UserID")) & "' and password='" & rtrim(request("password")) &
"'", connstrx, 3, 4
' more syntax below not relative to question
%>

will this be sufficient for the time being in preventing SQL Injection
until I have time to create new syntax and store procedures

 

[Michael Kujawa]

will this be sufficient for the time being in preventing SQL Injection
until I have time to create new syntax and store procedures

Changed the or's to and's but question still stands

if (instr(rtrim(request("UserID")),"'")=0 and
instr(rtrim(request("password")),"'")=0) and
(instr(rtrim(request("UserID")),"""")=0 and
instr(rtrim(request("password")),"""")=0) then

 

[Bob Barrows [MVP]]

This will suffice for a lazy or inexperienced hacker. For an experienced
hacker who is determined to break into your database/site, this will
probably slow him down by about 10 min. 20 min. tops.
Sorry.

http://www.sqlsecurity.com/DesktopDefault.aspx?tabid=23
http://www.nextgenss.com/papers/advanced_sql_injection.pdf
http://www.nextgenss.com/papers/more_advanced_sql_injection.pdf
http://www.spidynamics.com/papers/SQLInjectionWhitePaper.pdf

 

[Bob Barrows [MVP]]

<%
if request.form("submit")="Login" then
if len(rtrim(request("UserID")))>0 and
len(rtrim(request("Password")))>0 then
' line added to trap single - dbl quote
if (instr(rtrim(request("UserID")),"'")=0 or
instr(rtrim(request("password")),"'")=0) and
(instr(rtrim(request("UserID")),"""")=0 or
instr(rtrim(request("password")),"""")=0) then
rs.open "select * from FTSUsers where UserID='" &
rtrim(request("UserID")) & "' and password='" &
rtrim(request("password")) & "'", connstrx, 3, 4
' more syntax below not relative to question
%>

Oh! And you really should reduce the number of calls to Request
collections ... as well as specifying which form collection you are
accessing. Change "form" to "querystring" in the following example if
necessary:

dim userid, pwd
if request.form("submit") = "Login then
userid= rtrim(request.form("UserID"))
pwd=rtrim(request.form("Password"))
etc.

 

[Michael Kujawa]

This will suffice for a lazy or inexperienced hacker. For an experienced
hacker who is determined to break into your database/site, this will
probably slow him down by about 10 min. 20 min. tops.
Sorry.

http://www.sqlsecurity.com/DesktopDefault.aspx?tabid=23
http://www.nextgenss.com/papers/advanced_sql_injection.pdf
http://www.nextgenss.com/papers/more_advanced_sql_injection.pdf
http://www.spidynamics.com/papers/SQLInjectionWhitePaper.pdf

Ugh...

Well I added an ";" as a character to check for as well.
I know after reading the articles you posted this will not
prevent injection but will add a very small layer of defense
until I can get around to a full rewrite. As far as I can tell,
there has been no injection attempts and the site has been
active as is for 1 year.

<sigh>
I should have quoted higher. There are so many pages and
they did not use "any" includes.

Many thanks for the articles and the honest answer.

Mike K

 

[Michael Kujawa]

dim userid, pwd
if request.form("submit") = "Login then
userid= rtrim(request.form("UserID"))
pwd=rtrim(request.form("Password"))
etc.

Normally do that ie: variables to replace request calls.

... as well as specifying which form collection you are
accessing. Change "form" to "querystring" in the following example if
necessary:

hmmm is there any advantage to that?

I sort of figured if you do not have a mix of querystring values
and form post values you could use a generic request() without
specifying the type.

There have been times when I have seen querystring values coming
from the URL "as well as" values coming in from a form on the same
URL using post method. At which case you would have to use both
Request.QueryString() and Request.Form() ... correct?

But if it is just one or the other is there any practical reason to specify?

 

[Bob Barrows [MVP]]

Normally do that ie: variables to replace request calls.



hmmm is there any advantage to that?

Absolutely:
http://www.aspfaq.com/show.asp?id=2111

I sort of figured if you do not have a mix of querystring values
and form post values you could use a generic request() without
specifying the type.

You can, but then you force all the request collections to be searched
.... and that servervariables collection is pretty large.
Also, if you manage to duplicate a key in the servervariables
collection, you will spend hours trying to figure out why the variable
does not contain the value you expect it to contain.

There have been times when I have seen querystring values coming
from the URL "as well as" values coming in from a form on the same
URL using post method. At which case you would have to use both
Request.QueryString() and Request.Form() ... correct?
Yes.


But if it is just one or the other is there any practical reason to
specify?

See above.

 

[Michael Kujawa]

Absolutely:
http://www.aspfaq.com/show.asp?id=2111


See above.

Thanks again Bob,

I am the "out of laziness" as defined on the site.
I am a 1-2 finger per hand, looking at the keyboard
while typing typist. Any shortcuts in syntax I try that
work, I gravitate towards...

Although I would say I have pretty good speed for
using one to two fingers per hand.

BTW that site is a great resource.

Mike K

 

[Bob Barrows [MVP]]

Thanks again Bob,

I am the "out of laziness" as defined on the site.
I am a 1-2 finger per hand, looking at the keyboard
while typing typist. Any shortcuts in syntax I try that
work, I gravitate towards...

Although I would say I have pretty good speed for
using one to two fingers per hand.

BTW that site is a great resource.

Something not mentioned in that article:

dim vars
set vars = Request.Form

userid=vars("UserId")
etc.

How's that for efficiency that also caters to laziness?

 

[Michael Kujawa]

Something not mentioned in that article:

dim vars
set vars = Request.Form

userid=vars("UserId")
etc.

How's that for efficiency that also caters to laziness?

SWEET
Never thought of that

 

[Mike Brind]

Hi All,
I have been given a site to redo. In the process of looking at the code,
the live site is open to SQL injection. I know what needs to be done but
limited time right now to redo correctly. In the interm while I am rewriting
the site, will adding a few lines of code as below prevent SQL injection
until I have the time to rebuild the functions and move to stored
procedures.

Basically client side I added a onKeypress javascript routine
to look for ' or " and disallow in login fields

<script>
function checkcode()
{
if(event.keyCode==34 || event.keyCode==39){event.returnValue = false;}
}
</script>

ServerSide I then added an if else statement to trap if user has javascript
disabled

<snip>

Never rely on clientside validation as your first line of defence, with
server side as back up like this. Do it the other way round. I can
take a copy of your form and save it to my pc, then remove all
javascript from the source (and calls to javascript functions).
Finally, I'd change the form's action to a full http://www url. Then,
with javascript enabled, submit my revised version of your form -
bypassing both your lines of defence in about 2 minutes flat. And I'm
not an experienced hacker

 

[Michael Kujawa]

Never rely on clientside validation as your first line of defence, with
server side as back up like this. Do it the other way round. I can
take a copy of your form and save it to my pc, then remove all
javascript from the source (and calls to javascript functions).
Finally, I'd change the form's action to a full http://www url. Then,
with javascript enabled, submit my revised version of your form -
bypassing both your lines of defence in about 2 minutes flat. And I'm
not an experienced hacker

Looks I will have to make the time then to update at least the login form
right away. General consensus is it won't protect.

However I am not sure I follow about the form action. The page does not
have a form action pointing to any other page. The action is part of the
form
page and the form processing is done server-side.

So how could you remove the vbscript "if then" which is a safeguard against
invoking the recordset creation if a ' or " or ; is encountered in the
request.form()
value regardless if javascript is enabled or not?

Mike K

 

[Bob Barrows [MVP]]

Looks I will have to make the time then to update at least the login
form right away. General consensus is it won't protect.

However I am not sure I follow about the form action. The page does
not have a form action pointing to any other page. The action is part
of the form
page and the form processing is done server-side.

So how could you remove the vbscript "if then" which is a safeguard
against invoking the recordset creation if a ' or " or ; is
encountered in the request.form()
value regardless if javascript is enabled or not?

I think he's responding to your "ServerSide I then added an if else
statement to trap if user has javascript
disabled" statement, which seems to imply that you will only validate if
javascript is not enabled. Server-side validation should occur
regardless of whether javascript was enabled or not. A clever hacker
would easily cause his browser to report that javascript was enabled.
Actually, as Mike says, he wouldn't even have to. Using a debugger, the
hacker can cause your client-side functions to return any value he wants
them to.

 

[Michael Kujawa]

I think he's responding to your "ServerSide I then added an if else
statement to trap if user has javascript
disabled" statement, which seems to imply that you will only validate if
javascript is not enabled. Server-side validation should occur
regardless of whether javascript was enabled or not. A clever hacker
would easily cause his browser to report that javascript was enabled.
Actually, as Mike says, he wouldn't even have to. Using a debugger, the
hacker can cause your client-side functions to return any value he wants
them to.

--
Microsoft MVP -- ASP/ASP.NET
Please reply to the newsgroup. The email account listed in my From
header is my spam trap, so I don't check it very often. You will get a
quicker response by posting to the newsgroup.

Gotcha

I am definitely going to change the login page
and get rid of the very unsecure authentication
method tonight.

Thanks for all the insight Bob and Mike

 

[Mike Brind]

Looks I will have to make the time then to update at least the login form
right away. General consensus is it won't protect.

However I am not sure I follow about the form action. The page does not
have a form action pointing to any other page. The action is part of the
form
page and the form processing is done server-side.

If you do not specify a url in the action attribute of a form, the
default is that it posts to itself. So if myform on mypage.asp at
mysite.com is invoked like this:

<form method="post">

it will post to mypage.asp at http://www.mysite.com - in other words
http://www.mysite.com/mypage.asp. The server side code on mypage.asp
is waiting for some values to be posted to it so it can process them.
It has absolutely no way of knowing whether myform on mypage.asp was
the source of those values.

So how could you remove the vbscript "if then" which is a safeguard against
invoking the recordset creation if a ' or " or ; is encountered in the
request.form()
value regardless if javascript is enabled or not?

Because your OP said that the VBScript only kicked into play if the
browser had javascript disabled:

<Quote>
ServerSide I then added an if else statement to trap if user has
javascript
disabled
</Quote>

Clearly if the serverside code runs regardless of whether I have
javascript disabled or enabled, I can't get round the fact that it will
run, which should be your default position.

 

[Michael Kujawa]

<Quote>
ServerSide I then added an if else statement to trap if user has
javascript
disabled
</Quote>

Clearly if the serverside code runs regardless of whether I have
javascript disabled or enabled, I can't get round the fact that it will
run, which should be your default position.

Understood,
I sometimes word things incorrectly.
Just ask my better half...

firstline defense is vbscript serverside

 

[Adrienne Boswell]

Gazing into my crystal ball I observed "Bob Barrows [MVP]" <reb01501
@NOyahoo.SPAMcom> writing in

if you manage to duplicate a key in the servervariables
collection, you will spend hours trying to figure out why the variable
does not contain the value you expect it to contain.

I learned that one a long time ago, request("url") was reading from the
servervariables, and not the form collection, so I was getting
http://www.example.com instead of http://www.example.net.