|
it
newsgroups
|
|||||||||||||||||||||||
|
|||||||||||||||||||||||
SQL Injection Prevention Quickfix.. will it work
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" <nof at kujawas dot net> wrote in message Changed the or's to and's but question still standsnews: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 > 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 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 Michael Kujawa wrote: Show quote > 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]" <reb01501@NOyahoo.SPAMcom> wrote in message Ugh...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/advanced_sql_injection.pdf > http://www.nextgenss.com/papers/more_advanced_sql_injection.pdf > http://www.spidynamics.com/papers/SQLInjectionWhitePaper.pdf > 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 wrote:
Show quote > <% Oh! And you really should reduce the number of calls to Request> 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 > %> > 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. "Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message <snip>news:eh0ZOv8kGHA.5036@TK2MSFTNGP04.phx.gbl... > dim userid, pwd Normally do that ie: variables to replace request calls.> if request.form("submit") = "Login then > userid= rtrim(request.form("UserID")) > pwd=rtrim(request.form("Password")) > etc. > > hmmm is there any advantage to that?> ... as well as specifying which form collection you are >accessing. Change "form" to "querystring" in the following example if > necessary: 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? Michael Kujawa wrote:
Show quote > "Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message Absolutely:> 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? > http://www.aspfaq.com/show.asp?id=2111 > I sort of figured if you do not have a mix of querystring values You can, but then you force all the request collections to be searched> and form post values you could use a generic request() without > specifying the type. > .... 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 Yes.> 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? > See above.> But if it is just one or the other is there any practical reason to > specify? -- 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]" <reb01501@NOyahoo.SPAMcom> wrote in message <snip>news:%23zOs%23%238kGHA.1272@TK2MSFTNGP03.phx.gbl... > Absolutely: Thanks again Bob,> http://www.aspfaq.com/show.asp?id=2111 > > > But if it is just one or the other is there any practical reason to > > specify? > > See above. 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 Michael Kujawa wrote:
Show quote > "Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message Something not mentioned in that article:> news:%23zOs%23%238kGHA.1272@TK2MSFTNGP03.phx.gbl... > > <snip> >> Absolutely: >> http://www.aspfaq.com/show.asp?id=2111 >> >>> But if it is just one or the other is there any practical reason to >>> specify? >> >> 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. > 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. "Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message SWEETnews:%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? > Never thought of that Gazing into my crystal ball I observed "Bob Barrows [MVP]" <reb01501
@NOyahoo.SPAMcom> writing in news:#zOs##8kGHA.1272@TK2MSFTNGP03.phx.gbl: I learned that one a long time ago, request("url") was reading from the > 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. 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 Michael Kujawa wrote:
Show quote > Hi All, <snip>> 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 > 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
Show quote
"Mike Brind" <paxton***@hotmail.com> wrote in message Looks I will have to make the time then to update at least the login formnews: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 > 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 Michael Kujawa wrote:
Show quote > "Mike Brind" <paxton***@hotmail.com> wrote in message I think he's responding to your "ServerSide I then added an if else> 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? > 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.
Show quote
"Bob Barrows [MVP]" <reb01501@NOyahoo.SPAMcom> wrote in message Gotchanews: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. > > 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 Michael Kujawa wrote:
Show quote > "Mike Brind" <paxton***@hotmail.com> wrote in message If you do not specify a url in the action attribute of a form, the> 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. 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. > Because your OP said that the VBScript only kicked into play if the> 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? > 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
Show quote
"Mike Brind" <paxton***@hotmail.com> wrote in message Understood,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 > I sometimes word things incorrectly. Just ask my better half... firstline defense is vbscript serverside |
|||||||||||||||||||||||