|
Home > Archive > IIS ASP > June 2006 > SQL Injection Prevention Quickfix.. will it work
You are viewing an archived Text-only version of the thread.
To view this thread in it's original format and/or if you want to reply to
this thread please [click here]
| Author |
SQL Injection Prevention Quickfix.. will it work
|
|
| Michael Kujawa 2006-06-19, 1:22 pm |
| 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 2006-06-19, 1:22 pm |
|
"Michael Kujawa" <nof at kujawas dot net> wrote in message
news:uZwbNO8kGHA.3936@TK2MSFTNGP05.phx.gbl...
>
> 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] 2006-06-19, 1:22 pm |
| 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/adv...l_injection.pdf
http://www.nextgenss.com/papers/mor...l_injection.pdf
http://www.spidynamics.com/papers/S...nWhitePaper.pdf
Michael Kujawa wrote:
> 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
--
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.
| |
| Bob Barrows [MVP] 2006-06-19, 1:22 pm |
| Michael Kujawa wrote:
> <%
> 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.
--
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.
| |
| Michael Kujawa 2006-06-19, 1:22 pm |
|
"Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message
news:u%23JZxa8kGHA.4304@TK2MSFTNGP03.phx.gbl...
> 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/adv...l_injection.pdf
> http://www.nextgenss.com/papers/mor...l_injection.pdf
> http://www.spidynamics.com/papers/S...nWhitePaper.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 2006-06-19, 7:22 pm |
|
"Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message
news:eh0ZOv8kGHA.5036@TK2MSFTNGP04.phx.gbl...
<snip>
> 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] 2006-06-19, 7:22 pm |
| Michael Kujawa wrote:
> "Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message
> news:eh0ZOv8kGHA.5036@TK2MSFTNGP04.phx.gbl...
> <snip>
>
> 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.
--
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.
| |
| Michael Kujawa 2006-06-19, 7:22 pm |
| "Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message
news:%23zOs%23%238kGHA.1272@TK2MSFTNGP03.phx.gbl...
<snip>
> 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] 2006-06-19, 7:22 pm |
| Michael Kujawa wrote:
> "Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message
> news:%23zOs%23%238kGHA.1272@TK2MSFTNGP03.phx.gbl...
>
> <snip>
>
>
>
> 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?
--
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.
| |
| Michael Kujawa 2006-06-19, 7:22 pm |
|
"Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message
news:%234Y$kX9kGHA.2200@TK2MSFTNGP05.phx.gbl...
> 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 2006-06-19, 7:22 pm |
|
Michael Kujawa wrote:
> 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 :-)
--
Mike Brind
| |
| Michael Kujawa 2006-06-19, 7:22 pm |
|
"Mike Brind" <paxtonend@hotmail.com> wrote in message
news:1150749443.107952.257820@i40g2000cwc.googlegroups.com...
>
> 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 :-)
>
> --
> 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.
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] 2006-06-19, 7:22 pm |
| Michael Kujawa wrote:
> "Mike Brind" <paxtonend@hotmail.com> wrote in message
> news:1150749443.107952.257820@i40g2000cwc.googlegroups.com...
>
> 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.
--
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.
| |
| Michael Kujawa 2006-06-19, 7:22 pm |
| "Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message
news:OXTxcZ%23kGHA.5108@TK2MSFTNGP02.phx.gbl...
> 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 2006-06-19, 7:22 pm |
|
Michael Kujawa wrote:
> "Mike Brind" <paxtonend@hotmail.com> wrote in message
> news:1150749443.107952.257820@i40g2000cwc.googlegroups.com...
>
> 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.
--
Mike Brind
| |
| Michael Kujawa 2006-06-19, 7:22 pm |
| "Mike Brind" <paxtonend@hotmail.com> wrote in message
news:1150752757.477391.287830@h76g2000cwa.googlegroups.com...
> <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.
>
> --
> Mike Brind
>
Understood,
I sometimes word things incorrectly.
Just ask my better half...
firstline defense is vbscript serverside
| |
| Adrienne Boswell 2006-06-20, 1:36 am |
| Gazing into my crystal ball I observed "Bob Barrows [MVP]" <reb01501
@NOyahoo.SPAMcom> writing in news:#zOs##8kGHA.1272@TK2MSFTNGP03.phx.gbl:
> 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.
--
Adrienne Boswell at Home
Arbpen Web Site Design Services
http://www.cavalcade-of-coding.info
Please respond to the group so others can share
|
|
|
|
|