• Hans (unregistered)

    Why is a triangular wheel bad? It has one less bump per revolution compared to a square wheel!

  • (nodebb)

    Triangular wheels are good! See 'The 3 cornered wheel' by Poul Anderson

  • Rob (unregistered)

    It doesn't even protect against mix cases like DrOp...

  • (nodebb)

    Given how utterly dysfunctional this code is, I think it was written out of malicious or perhaps lazy compliance. The manager said, write code to protect against SQL injection; the developer did write. The manager didn't say, test it, debug it, or anything like that.

  • dusoft (unregistered)

    What are these 503 and 500 errors? A big WTF to the readers? ;-)

  • Hanzito (unregistered)

    I can already imagine the "architect" behind this code explain why passwords, user names, etc. cannot contain anything except ASCII letters and digits.

    Changing that would make our site

    • insecure
    • vulnerable to the h4x0r5

    At that point in the PowerPoint presentation, there would be an explanation of 1335 and how hackers use it to scramble their communications, and management would nod wisely and praise themselves happy with so much security.

  • (nodebb)

    also, if something bad doesn't get detected, it happily slips through. I think this should be patented...

  • (nodebb)

    And then have a column named "selection" and break the code [just happened in the real world in the past 6 months]

  • I dunno LOL ¯\(°_o)/¯ (unregistered)

    This kind of bullshit is why WTFs happen. A few months ago I tried to upload a text file of commented assembly language source code to a web board as a file attachment, but fucking cloudflare flagged it as a security violation because it had a "bad word" in it. I think it took me three times before I figured out that I had to put it in a zip file.

    Not everything gets interpreted as SQL, you assholes. The problem is that there are morons out there who will go out of their way to throw anything and everything into a SQL command anyhow. It's a corollary of "the universe can always create a bigger idiot".

  • MaxiTB (unregistered)

    If I got a house for anytime I have seen code like that against SQL injections at a customer... Just use query parameters, even PHP has them since v3 for all providers and PDO even unified everything years ago.

  • Kleyguerth (github)

    This happened in my very first non-intern job. I found the injection vulnerability, explained to the seniors how it could be easily exploited (through the password field) and they "fixed" it just like this article, banning SQL keywords. Bonus points for the solution as one of the business analysts had the word "from" in their password and got locked out of the system.

  • Loren Pechtel (unregistered) in reply to Kleyguerth

    Yeah, my first thought was a mandate from above to ban the keywords--the incorrect solution was dictated, not designed by a programmer. An awful lot of managers aren't good at realizing they're going at it the wrong way and not open to underlings telling them they're going at it the wrong way.

  • (nodebb)

    Code like this might be ok if it sat there, never being called. Invite the clueless managers to a technical review so they could nod wisely and report upwards that Something Has Been Done. Ideally by "sit there" it should be sitting in a suitably separate directory "for security", not anywhere where it might end up in a code repo.

  • Miles O'Keefe (unregistered)
    Comment held for moderation.
  • xtal256 (unregistered) in reply to Loren Pechtel
    Comment held for moderation.
  • NeverConfusedForNaught (unregistered) in reply to Mr. TA
    Comment held for moderation.
  • (nodebb)

    Some of the code in there looks like it hasn't been touched in twenty years, but all of it looks bad. (Like: firing off an arbitrary die() in the middle of your page says you've never had to write something that doesn't break.) I don't understand the penchant for putting a return value into a variable solely for the sake of returning the variable's value (still less calling said variable $newvar).

    Then there is the matter of using preg_split to split the string into individual characters to see if any of them are strings like "DELETE" (no, none of them will be). But that part of the code isn't going to match anything anyway because of the PREG_SPLIT_OFFSET_CAPTURE flag. With that flag set, what is returned is not an array of characters, but an an array of [character, string position] pairs (used to identify where in the string each match occurred).

    Not that you'd notice of course, since the only characters that might have matched anything have already been stripped out.

  • twist (unregistered)

    This is definitely one of those times the developer is making PHP look bad and not PHP being bad itself.

  • (nodebb) in reply to Watson

    Putting a return value into a variable just to return it in the next statement? Sometimes it's nice while debugging. You step through the code, you see what value got computed and you can tweak it if you need to test the rest of the code with a different return value.

  • (nodebb) in reply to Jenda

    This person never did any debugging.

  • nasch (unregistered)
    Comment held for moderation.
  • löchlein deluxe (unregistered)

    My new favourite conspiracy theory: client-side password hashing is done to make sure the stored password only consists of hexits, but you tell mgmt something about offloading computing costs to the client and how it helps with DDOS.

  • nz (unregistered)

    Sure this is a WTF but not the one the comments imply. Did anyone notice the array is iterated by value? So the code doesn't actually modify $_POST. It only checks the values against a blacklist (only part of which works actually as it checks sanitized values). Thus the code is virtually harmless... until someone would try to fix it. Oops.

Leave a comment on “Anti-Injection”

Log In or post as a guest

Replying to comment #:

« Return to Article