SQL Injection Prevention Quickfix.. will it work

Discussion in 'ASP General' started by Michael Kujawa, Jun 19, 2006.

  1. 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, Jun 19, 2006
    #1
    1. Advertising

  2. "Michael Kujawa" <nof at kujawas dot net> wrote in message
    news:...
    >
    > 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
    Michael Kujawa, Jun 19, 2006
    #2
    1. Advertising

  3. 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:
    > 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], Jun 19, 2006
    #3
  4. 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.
    Bob Barrows [MVP], Jun 19, 2006
    #4
  5. "Bob Barrows [MVP]" <> wrote in message
    news:u%...
    > 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, Jun 19, 2006
    #5
  6. "Bob Barrows [MVP]" <> wrote in message
    news:...
    <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?
    Michael Kujawa, Jun 19, 2006
    #6
  7. Michael Kujawa wrote:
    > "Bob Barrows [MVP]" <> wrote in message
    > news:...
    > <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?
    >


    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.
    Bob Barrows [MVP], Jun 19, 2006
    #7
  8. "Bob Barrows [MVP]" <> wrote in message
    news:%23zOs%23%...

    <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.

    Mike K
    Michael Kujawa, Jun 19, 2006
    #8
  9. Michael Kujawa wrote:
    > "Bob Barrows [MVP]" <> wrote in message
    > news:%23zOs%23%...
    >
    > <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.
    >

    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.
    Bob Barrows [MVP], Jun 19, 2006
    #9
  10. "Bob Barrows [MVP]" <> wrote in message
    news:%234Y$...
    > >

    > 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
    Michael Kujawa, Jun 19, 2006
    #10
  11. Michael Kujawa

    Mike Brind Guest

    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
    Mike Brind, Jun 19, 2006
    #11
  12. "Mike Brind" <> wrote in message
    news:...
    >
    > 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
    Michael Kujawa, Jun 19, 2006
    #12
  13. Michael Kujawa wrote:
    > "Mike Brind" <> wrote in message
    > news:...
    >>
    >> 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?
    >


    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.
    Bob Barrows [MVP], Jun 19, 2006
    #13
  14. "Bob Barrows [MVP]" <> wrote in message
    news:OXTxcZ%...
    > 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
    Michael Kujawa, Jun 19, 2006
    #14
  15. Michael Kujawa

    Mike Brind Guest

    Michael Kujawa wrote:
    > "Mike Brind" <> wrote in message
    > news:...
    > >
    > > 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.


    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
    Mike Brind, Jun 19, 2006
    #15
  16. "Mike Brind" <> wrote in message
    news:...

    > <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
    Michael Kujawa, Jun 19, 2006
    #16
  17. Gazing into my crystal ball I observed "Bob Barrows [MVP]" <reb01501
    @NOyahoo.SPAMcom> writing in news:#zOs##:

    > 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
    Adrienne Boswell, Jun 20, 2006
    #17
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Mike Curry
    Replies:
    1
    Views:
    489
    Anon-E-Moose
    Jul 23, 2004
  2. Carl Gilbert

    Image padding prevention / issue

    Carl Gilbert, Dec 6, 2004, in forum: HTML
    Replies:
    7
    Views:
    514
    Toby Inkster
    Dec 8, 2004
  3. greenvelvet

    selective hotlink prevention

    greenvelvet, Oct 4, 2005, in forum: HTML
    Replies:
    1
    Views:
    420
    greenvelvet
    Oct 4, 2005
  4. cool2005

    Duplicate submission prevention

    cool2005, Sep 26, 2006, in forum: Java
    Replies:
    3
    Views:
    1,934
    cool2005
    Sep 26, 2006
  5. Simon Wigzell

    ASP and SQL Injection prevention

    Simon Wigzell, Mar 7, 2006, in forum: ASP General
    Replies:
    1
    Views:
    125
    Bob Barrows [MVP]
    Mar 7, 2006
Loading...

Share This Page