• jvm (unregistered)

    engfeh

  • Amazinderek (unregistered) in reply to jvm

    Wow!  Talk about a gift from God for anyone who wants to royally screw that application!

    Let's see now, manually changing my cookie to 'drop database xyz'...

  • (cs)

    That sure had me tossing cookies all over my desk. Eugh.

  • Amazinderek (unregistered) in reply to Amazinderek
    Anonymous:
    Wow!  Talk about a gift from God for anyone who wants to royally screw that application!

    Let's see now, manually changing my cookie to 'drop database xyz'...


    To clarify, I have inferred that this system will, at some later point, use this cookie-stored SQL string at face value since there is no other reason to create the cookie.
  • (cs)

    Oh... your... gods!

    I refuse to believe that's production code.

    Who needs creative escape sequences in the input variables when one can just edit the sql?

  • (cs)

    wibble...

    isn't there some rule against putting your logic into SQL statements?

    I'm quite impressed (in a horrified way) at how he's managed to harness the limited powers of SQL to do all of that. I think, just from this one snippet, that SQL should either: be turned into a full-blown turing-complete language, to soften the pain of people implementing their main logic in it; or have a lot of its functionality stripped out (trim(), char() and replace() spring to mind from that example), to prevent said pain ever happening in the first place.

  • Rob (unregistered)

    This is pretty sadistic.

    I can only imagine the thought process behind this:

    Jesus..this is a nightmare

    Hardcoded SQL statement....in a cookie, I'm guessing security isn't a priority with this system

    What hat the f* is up with all the "rtrim(ltrim(replace(replace(replace(replace(isnull", I'm guessing performance isn't a priority with this system

    Stored procedures are for wusses!!! Real men hardcode their SQL using one HUGE string into a cookie

  • (cs) in reply to Irrelevant

    ...and now I've clicked "post", I notice the main point of the post.

    oh, eris. O.o

  • (cs)

    Okay, apologists. Let's hear a rationalization for this one. If you feel that session variables are the tool of the devil, you must acknowledge that this is the Giant Space Laser of the devil.

  • (cs) in reply to Irrelevant

    Irrelevant:
    ...and now I've clicked "post", I notice the main point of the post. oh, eris. O.o

    lol - You beat me to pointing out that your previous post was pretty much exactly what your nic says WRT the WTF [;)]

    As for the WTF, after looking at it my face is on fire.

  • (cs)

    Ahmmm.... uhhnh... mmmmmff. fnff... umm. no.

    It BURNS.. it BURNS!!  Why, oh why would any sane person do this.

    1. SQL Statement from hell. I'm too lazy to decode what it does, someone give me the jist.
    2. Store SQL statement in a cookie named SQL. Brilliant!
    3. Redirect to a cleverly named "callquery.asp" page. Amazing!

    Why would someone have an ASP page that does nothing but run a query passed in by the client?? at what point did they think that was a good idea? Were they dropped on their head at birth? Were they trained to be ASP developers by the likes of ITT Tech? Do they have delusions of grandeur (that what they did was actually a good and correct way of doing things)? Why didn't their boss take them out behind the shed and end their (the boss'?) misery. How do people like this secure jobs in the IT world.

    Raw SQL passed in by the client = so incredibly stupid that even an untrained monkey would know better!
  • (cs)

    <font size="1">I counted 290 string operations.
    Obviously, this is part of a database load test utility...

    cripes.
    </font>

  • (cs) in reply to Mike R
    Mike R:

    Raw SQL passed in by the client = so incredibly stupid that even an untrained monkey would know better!


    My monkey programmer colleague found that remark incredibly degrading and deeply offensive to monkeys. :P
  • (cs) in reply to mizhi
    mizhi:

    My monkey programmer colleague found that remark incredibly degrading and deeply offensive to monkeys. :P


    I retract my statement then: A caveman would know better [;)]
  • (cs)

    Geez... There's so many fewer keystrokes in
        Session("sql") =
    than in
        Response.Cookie("sql") =

  • (cs)

    ugh... such pain... I can't believe it...

    and besides the obvious security risks/stupidity.... what's up with 

    	replace(replace(replace(replace(replace(cast(isnull(cq.problem,'')

    showing up all through the sql...


  • Anonymous (unregistered) in reply to mizhi

    mizhi:
    Mike R:

    Raw SQL passed in by the client = so incredibly stupid that even an untrained monkey would know better!


    My monkey programmer colleague found that remark incredibly degrading and deeply offensive to monkeys. :P

    If your colleague is a monkey, I was wondering what are you!

  • (cs) in reply to Anonymous
    Anonymous:

    mizhi:
    Mike R:

    Raw SQL passed in by the client = so incredibly stupid that even an untrained monkey would know better!


    My monkey programmer colleague found that remark incredibly degrading and deeply offensive to monkeys. :P

    If your colleague is a monkey, I was wondering what are you!

    In this country, a monkey's colleague is called a Senior Developer.

  • (cs) in reply to Anonymous
    Anonymous:

    mizhi:
    Mike R:

    Raw SQL passed in by the client = so incredibly stupid that even an untrained monkey would know better!


    My monkey programmer colleague found that remark incredibly degrading and deeply offensive to monkeys. :P

    If your colleague is a monkey, I was wondering what are you!



    Zookeeper.
  • Anonymous (unregistered) in reply to rogthefrog
    rogthefrog:
    Anonymous:

    mizhi:
    Mike R:

    Raw SQL passed in by the client = so incredibly stupid that even an untrained monkey would know better!


    My monkey programmer colleague found that remark incredibly degrading and deeply offensive to monkeys. :P

    If your colleague is a monkey, I was wondering what are you!

    In this country, a monkey's colleague is called a Senior Developer.

    So all of your colleagues must be senior developers!

  • (cs)

    The REAL WTF here is the use of varchar.  Everybody knows you have to use varchar2.  (It supports strings, as well as true, false, and true2.)

    Also, char(130) and char(145)?  WTF character set is that supposed to be?

  • Chris F (unregistered)

    I didn't send the destination page, but in a nutshell it extracts the SQL from the cookie, executes verbatim, and displays the results. This brain-damaged idiom is found all throughout the application.

  • (cs) in reply to Chris F

    At the year end WTFY awards, I think we have our first nominee for "WTF of the Year" !

     

  • (cs) in reply to loneprogrammer
    loneprogrammer:
    The REAL WTF here is the use of varchar.  Everybody knows you have to use varchar2.  (It supports strings, as well as true, false, and true2.)

    Also, char(130) and char(145)?  WTF character set is that supposed to be?


    char(130)==é
    char(145)==ç

    Was the original developer French?  If so they're also missing char(138)==è, char(140)==î, char(136)==ê, char(131)==â, char(133)==à, char(147)==ô, etc.  Using ALT-# combinations is fun.
  • (cs)

    Ah... Brainfuc* student... [:^)]

  • delta407 (unregistered)

    Okay, I think I've managed to decode this. This appears to be the "find" functionality of some horrible ticketing system. It starts out innocently enough:

    select
        cq.InquiryID,
        cq.userid,
        cq.assignedto,
        cq.customerid,
        cq.InquiryDate,
        cq.InquiryType,
        cq.Priority,

    Then, out of nowhere:
        isnull((select ct.typename from CustomerDB.dbo.InquiryTypes ct where ct.typeid = isnull(cq.inquirytype,0)),'') as inquirytypename,

    Personally, I would have implemented this as a LEFT JOIN rather than a scalar subselect. Unless SQL Server magically rewrites queries more than I remember, the above results in a separate query against the InquiryTypes table per row in the result set.

    And again:
        isnull((select cs.statusname from CustomerDB.dbo.InquiryStatuses cs where cs.statusid = isnull(cq.inquirystatus,0)),'') as inquirystatusname,

    And again:
        isnull((select pi.priorityname from CustomerDB.dbo.InquiryPriorities pi where pi.priorityid = isnull(cq.priority,0)),'') as priorityname,

    Okay. Now, look at this:
        replace(replace(replace(replace(replace(cast(isnull(cq.problem,'') as varchar(8000)),char(44),char(130)),char(39),char(145)),char(10),' '),char(13),' '),char(92),char(47)) as problem,

    This takes cq.problem (or, if null, an empty string), casts it to a varchar (so replace() can work on it, presumably), and then replaces ASCII character 44 with 130, 39 with 145, 10 with ' ', 13 with ' ', and 92 with 47. Put another way, this replaces apostrophes with single "smart quotes", commas with Unicode character 201A, "Single Low-9 Quotation Mark", CR/LFs with spaces, and backslashes with forward slashes. The only reason for this I can think of is that these values will be injected directly into a SQL statement at some later point -- the quotes and commas look similar, but would be parsed differently. This would also explain the backslashes.

    Ick.

    Moving on, we have another LEFT JOIN candidate:
        isnull((select usr.username from CustomerDB.dbo.users usr where usr.userid = isnull(cq.userid,0)),'') as UserName,

    This next one is great:
        rtrim(ltrim(
            replace(upper(left(ltrim(rtrim(us.FirstName)),1))
                + right(ltrim(rtrim(us.FirstName)),(len(ltrim(rtrim(us.FirstName)))-1)
                +' '
                +upper(left(ltrim(rtrim(us.LastName)),1))
                +right(ltrim(rtrim(us.LastName)),len(ltrim(rtrim(us.LastName)))-1
            ),char(39),char(145))
        )) as AssignName,

    This uppercases the first letter of the users' first and last names, while lowercasing the rest and joining them with spaces. In PHP, this would be ucwords(strtolower("$first $last")).

        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.fname,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as FirstName,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.lname,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as LastName,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.address1,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as Address1,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.address2,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as Address2,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.apt,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as Apt,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.city,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as City,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.state,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as State,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.countryid,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as Country,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.postalcode,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as PostalCode,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.homephone,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as HomePhone,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.officephone,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as OfficePhone,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.fax,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as Fax,
        rtrim(ltrim(replace(replace(replace(replace(isnull(cc.email,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as CEmail,
        rtrim(ltrim(replace(replace(replace(replace(isnull(us.email,''),char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as UEmail,
        rtrim(ltrim(replace(replace(replace(replace(de.department,char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as Department,
        rtrim(ltrim(replace(replace(replace(replace(lo.locations,char(39),char(145)),char(44),char(130)),char(10),' '),char(13),' '))) as Location,

    Beautiful! Apparently backslashes aren't problematic in these fields. Maybe they're validated on input.

    from CustomerDB.dbo.CustomerInquiries cq
    left outer join CustomerDB.dbo.users us ON cq.assignedto = us.userid
    inner join CustomerDB.dbo.customers cc on cq.customerid = cc.customerid
    left outer join CustomerDB.dbo.departments de on us.department = de.id
    left outer join itticket.dbo.locations lo on us.location = lo.id

    Oh, look! Joins! IMAGINE!

    If numQueueID > 0 Then
      'string concatenation, field name, value

      txtFind = txtFind & limitfind(txtFind, "cq.inquiryid", numQueueID)
    End If

    txtFind = txtFind & limitfind(txtFind, "cq.userid", numUserID)
    txtFind = txtFind & limitfind(txtFind, "cq.assignedto", numAssignedTo)
    txtFind = txtFind & limitfind(txtFind, "cq.customerid", numCustomerID)
    txtFind = txtFind & limitfind(txtFind, "cq.inquiryqueue", strTicketQueue)
    txtFind = txtFind & limitfind(txtFind, "cq.inquirytype", numCallTypeID)
    txtFind = txtFind & limitfind(txtFind, "cq.inquirystatus", numCallStatusID)
    txtFind = txtFind & limitfind(txtFind, "cq.priority", numCallPriorityID)
    txtFind = txtFind & limitfind(txtFind, "cq.uemail", strUEmail)
    txtFind = txtFind & limitfind(txtFind, "de.department", strDepartment)
    txtFind = txtFind & limitfind(txtFind, "lo.locations", strLocation)
    All I can figure with this retarded syntax is that limitFind() will prepend an "AND" to its output if txtFind is non-empty. Now, I can't think of why you'd bother passing in txtFind and then  also force the caller to concatentate them outside of limitFind(), but by looking at the rest of the code, it's obvious these are beings with a totally new perspective. ;-)

    This makes me want to cry.

  • Thag The Caveman (unregistered) in reply to Mike R
    Mike R:
    mizhi:

    My monkey programmer colleague found that remark incredibly degrading and deeply offensive to monkeys. :P


    I retract my statement then: A caveman would know better [;)]


    Thag offended. Thag better programmer than untrained monkey.
  • (cs) in reply to delta407

    Anonymous:
    Okay, I think I've managed to decode this. This appears to be the "find" functionality of some horrible ticketing system. It starts out innocently enough:

    select
        cq.InquiryID,
        cq.userid,
        cq.assignedto,
        cq.customerid,
        cq.InquiryDate,
        cq.InquiryType,
        cq.Priority,

    Then, out of nowhere:
        isnull((select ct.typename from CustomerDB.dbo.InquiryTypes ct where ct.typeid = isnull(cq.inquirytype,0)),'') as inquirytypename,

    Personally, I would have implemented this as a LEFT JOIN rather than a scalar subselect. Unless SQL Server magically rewrites queries more than I remember, the above results in a separate query against the InquiryTypes table per row in the result set.

    [...]


    This makes me want to cry.

    Wow. Good job ;-)

    Thought I'd clarify, good RDBMS (SQL Server Included) will magically cause a join to happen with sub-queries like that. Play around with Northwind and Joins vs. Subqueries and see identical execution plans.

  • OneFactor (unregistered)

    Alex Papadimoulis:
    As you read this short introduction, I'm guessing that you can see today's code in your peripheral vision and are wondering, Tossing Your Cookies? What does that have to do with a huge, ugly SQL statement? I'll let the combination of "Cookies" and "SQL" sink in for a moment ........ ahh, there it is. And yes, it's exactly what your thinking. It's

    ebeh ebeh ebeh

    Alex, you read my mind. Cookies. SQL statement. Uh oh. Please no. Oh dear.

    Alex, if you find yourself by my side during my dying moments, please tell me all the stories you posted here are hoaxes. It will restore my faith and give me strength to face the next life bravely.

  • katre (unregistered)

    The best part about this is the sure knowledge that someone, somewhere, complained about the performance, and was told "Oh, you need a quad processor dedicated server for this app, sorry."

  • (cs)

    [:|][blink, blink]  My EYES!  They are burning up!!  What the Fu*k is up with this SQL Injection frick'n nightmare of... OMG my brain just shut down trying to decode this horrific peice of Sh*t! [:S][:'(]

  • The other Thag (unregistered) in reply to Thag The Caveman

    Anonymous:
    Mike R:
    mizhi:

    My monkey programmer colleague found that remark incredibly degrading and deeply offensive to monkeys. :P


    I retract my statement then: A caveman would know better [;)]


    Thag offended. Thag better programmer than untrained monkey.

    Would that be Thag "the Thag" Thag?

  • (cs) in reply to cm5400

    "The only reason for this I can think of is that these values will be injected directly into a SQL statement at some later point"

    Ohh, my. So, we're protecting from SQL injection attacks, while simultaneously storing raw SQL in a cookie and redirecting to a page that reads that raw sql and executes it verbatim. Makes sense. Was this person high on something when he developed this?

  • (cs)

    wow, talk about built in security.  any time they fire you, you snapshot the database and then nuke it to the ground and offer to sell it back to them.

    Ok, fine, its not security for the company, but why is it always about them???

    Oh, right, FBI bursting through your door is hell on the security deposit.

  • (cs)

    Has anyone noticed that this executes the query once just for testing whether it has results, and then ANOTHER time on the CallQuery.asp page?

    Talk about efficiency...

  • (cs) in reply to Xepol
    Xepol:

    wow, talk about built in security.  any time they fire you, you snapshot the database and then nuke it to the ground and offer to sell it back to them.

    Ok, fine, its not security for the company, but why is it always about them???

    Oh, right, FBI bursting through your door is hell on the security deposit.



    This assumes he would even know how to do that.  I have my doubts.
  • Rob (unregistered) in reply to mizhi
    mizhi:
    Anonymous:

    mizhi:
    Mike R:

    Raw SQL passed in by the client = so incredibly stupid that even an untrained monkey would know better!


    My monkey programmer colleague found that remark incredibly degrading and deeply offensive to monkeys. :P

    If your colleague is a monkey, I was wondering what are you!



    Zookeeper.

    I prefer the term "Director of Simian Relations"

    I put this as my job title in the employee database at my previous company.  The funny thing is, nobody noticed.

  • (cs)
    1. Load in cookie SQL.txt
    2. Log in as the SA
    3. Redirect to CallQuery.asp
    4. ???
    5. Profit!!!!!
  • (cs)

    Isn't there a 4000 character limit on cookies?

  • monkey (unregistered) in reply to oSand
    oSand:
    Isn't there a 4000 character limit on cookies?


    sure is
  • (cs)

    Judging by this guy's programming skills, does anyone wanna guess what closeconnection() does at the end?

  • monkey (unregistered) in reply to Manni

    I like the way they have left all the 'logic' in place so it's easy to see whats going on

      "rtrim(ltrim(replace(replace(replace(replace(isnull(cc.fname,''),char(39),char(145)),
        char(44),char(130)),char(10),' '),char(13),' '))) as FirstName, "

    there are just too many people around who do stupid things like write a user defined function called something like cleanData and then write something like

    "cleanData(cc.fname) AS FirstName"

  • (cs) in reply to Maurits

    Maurits:
    Geez... There's so many fewer keystrokes in
        Session("sql") =
    than in
        Response.Cookie("sql") =

    I'm assuming this is a joke. Otherwise, you are most likely the guy who wrote a lot of the code I'm maintaining today.

    In the app I maintain, for one particular report, it would stick the SQL in the Session object. Then when the user clicked a "download" button on the page, it would execute Session("SQL") to rerun the same report so it could format it as CSV. Guess what happened if the user went on their lunch break (let's say, oh, I don't know, a 20 minute lunch break) between viewing the report and downloading it.

     

  • (cs) in reply to Manni
    Manni:
    Judging by this guy's programming skills, does anyone wanna guess what closeconnection() does at the end?

    I'd guess it opens a new connection to the database to check if it's still online, then due to a programming error, tries to redirect the web browser to CloseConnection.asp but fails, leaving both connections still open.

  • (cs)

    Call me a skeptic, but I refuse to believe somebody would store a raw sql statement in a cookie. People with access to a db don't do this sort of thing.

    Do they?

    The world is ending.

  • monkey (unregistered) in reply to loneprogrammer
    loneprogrammer:
    Manni:
    Judging by this guy's programming skills, does anyone wanna guess what closeconnection() does at the end?

    I'd guess it opens a new connection to the database to check if it's still online, then due to a programming error, tries to redirect the web browser to CloseConnection.asp but fails, leaving both connections still open.


    it's where SQL Server comes into it's own you can just leave all those connections open, Access was such  a drag only allowing 20 or so before dying on you. Saves on on all that  typing ;p
  • (cs) in reply to rogthefrog
    rogthefrog:

    Call me a skeptic, but I refuse to believe somebody would store a raw sql statement in a cookie. People with access to a db don't do this sort of thing.

    Do they?

    The world is ending.



    I know it's hard to believe, but I know for a fact that this code is authentic.
  • (cs) in reply to A Wizard A True Star
    A Wizard A True Star:

    Maurits:
    Geez... There's so many fewer keystrokes in
        Session("sql") =
    than in
        Response.Cookie("sql") =

    I'm assuming this is a joke. Otherwise, you are most likely the guy who wrote a lot of the code I'm maintaining today.

    In the app I maintain, for one particular report, it would stick the SQL in the Session object. Then when the user clicked a "download" button on the page, it would execute Session("SQL") to rerun the same report so it could format it as CSV. Guess what happened if the user went on their lunch break (let's say, oh, I don't know, a 20 minute lunch break) between viewing the report and downloading it.

     



    No, not a joke... I have done this a couple of times...

    If the session times out, you display a "your session timed out" error.

    You can avoid session timeouts by having a
    <iframe src="/session-keepalive.asp" height=1 width=1>
    anywhere on the page, where session-keepalive.asp has an HTTP Refresh header of 60 * 15 seconds.  This will keep their session alive for as long as their browser is open to the page.
  • (cs) in reply to Maurits

    you have to be kidding me. and what if they dont load iframes?

  • (cs) in reply to Sparr
    Sparr:
    you have to be kidding me. and what if they dont load iframes?


    I'll let you figure that one out. :)

Leave a comment on “Tossing Your Cookies”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article