• P (unregistered)

    I initially thought they "scrubbed" the input twice because they did something like replacing all occurrences of select to empty string, which would allow injection by seselectlect, so they naturally applied the quickest "fix".

    The WTF turns out to be very underwhelming instead.

  • PenguinF (unregistered)

    Still not careful enough (before Ted C came along): REPLACE(@input, '"', '"')

  • bvs23bkv33 (unregistered)

    once i was using mysql_real_escape_string to concatenate string values into insert statement, but then i decided to use host variables and things went much better

  • ray10k (unregistered)

    By calling it twice, doesn't that mean that per example every @ gets turned into \@? Wonder if they had any trouble with duplicate backslashes having to be filtered out later.

  • ray10k (unregistered) in reply to ray10k

    Formatting tripped me up, I meant "every @ gets turned into \\@?"

  • Foo AKA Fooo (unregistered)

    "Nobody wants to have a Bobby Tables moment in their database. So we need to to sanitize our inputs."

    Huh? I hope that's sarcasm, or what's become of prepared statements?

  • (nodebb)

    There's a pizza chain in Toronto called Pizza Pizza. I think we should designate an official design pattern for this snippet and call it the Replace Replace pattern: tastes way worse than pizza, and still contributes to the obesity epidemic.

  • Reginald P. Smithington (unregistered)

    This isn't so bad. We have developers who would do this but make sure every replace was saved to a different variable.

  • TruePony (unregistered)

    It's good OO practice to favor composition over inheritance. The original author simply took it to the next level and favored composition over everything else.

  • Scott (unregistered)

    Kind of what ray10k is saying. If somebody provided what is (apparently, to this system) properly-escaped strings, won't this double-escape them and fail?

  • RLB (unregistered) in reply to Foo AKA Fooo

    Huh? I hope that's sarcasm, or what's become of prepared statements?

    Hit the nail on the head. TRWTF is the hordes of programmers, supposed professionals and supposed critics of other professionals, who in 2019 still haven't heard of bound parameters.

  • Little Bobby Tables (unregistered) in reply to Mr. TA

    "Yields falsehood when preceded by its quotation" yields falsehood when preceded by its quotation.

  • I can be a robot if you want me to be (unregistered)

    If the query is more than three years old then it was written before string_escape was built-in.

  • Black Mantha (unregistered)

    I like the bonus detail that double-quotes get replaced with double-quotes.

  • (nodebb)
    1. We start with the RDBMS user permissions for the app only allowing DML and not DDL
    2. In QA and Prod, only DBAs know the admin credentials.
    3. App services (on server) use bound params and a JDBC layer (Spring) that helps weeding out SQL injection.
    4. Client makes no direct calls to the RDBMS.
  • eric bloedow (unregistered) in reply to Mr. TA

    reminds me of an old MAD comic: there's a bunch of pizza places side-by-side, the first on says "best pizza in the city", the next one says "best pizza in the state", then "best pizza in the country", then "best pizza in the world"...and the last one says "best pizza on the block." and the last one is getting the most business!

  • Ted C (unregistered) in reply to I can be a robot if you want me to be

    It was written this year.

  • Ted C (unregistered) in reply to RLB

    Yeah. I would have preferred to be given the time to rewrite the entire way the app interfaces with the DB. I was given 2 hours to fix the bug.

  • VI (unregistered)

    Wait, isn't doing the sanitation in SQL already too late, regardless of which function you use?

  • (nodebb) in reply to eric bloedow

    Found that pizza cartoon: https://www.madmagazine.com/sites/default/files/imce/MAD-Magazine-Dave-Berg-Pizza-1_528b815965db18.69558913.jpg

  • Perri Nelson (unregistered) in reply to Mr. TA

    If I recall correctly a somewhat diminutive cartoonish "Caesar" says "Pizza Pizza" at the end of the commercials he appears in for the his eponymous pizza franchise.

  • Mr Parameter Binding guy (unregistered)

    I don't get it... the documentation I found says STRING_ESCAPE currently only supports JSON. The article mentions sanitizing inputs. What is the point of JSON encoding data on input? I would only expect to see this method used in a stored proc that is designed to return JSON encoded data. And then I would question what business is it of a database to be returning JSON (or HTML or XML for that matter).

  • BobE (unregistered) in reply to Mr. TA

    nine-six-seven-eleven-eleven. . . I haven't lived in Toronto for nearly forty years. If only I could remember some of my university lectures so well.

  • Sz (unregistered)

    I think STRING_ESCAPE is only available in SQL Server 2016+ and so maybe the function was written before that ?

  • RLB (unregistered) in reply to Ted C

    That may be an excuse for you, but not for your employer.

  • Ted C (unregistered) in reply to Mr Parameter Binding guy

    Yes, there is JSON involved. This may not be a rabbit hole you want to go down

  • Angela Anuszewski (google) in reply to Mr. TA

    Oh, why did you mention Pizza Pizza?!? Grr. Now I have that damned song stuck in my head.

    One, Two, Three, Four!

  • symbiont (unregistered)

    shouldn't single quote be escaped at the very least?

  • (nodebb) in reply to ray10k

    Perhaps somewhere along the line someone is trying to unescape this mess, thereby turning double backslash into single backslash, resulting in the original string now being properly escaped?

Leave a comment on “The Replacements”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article