• Industrial Automation Engineer (unregistered)

    Hungarian Notation?

  • (nodebb)

    Doesn't IndexOf return the location of the first occurrence of the string that you're searching for? So wouldn't this function cut off everything after the first space in the string?

    No wonder the function is never called; it breaks pretty much any query you pass to it.

  • Michael R (unregistered)

    ThRiD. Take this case-sensitive beauty.

  • (nodebb)

    I bet they stopped calling it because a some "Bob von Something" (or the local equivalent) complained that their family name was truncated to "von" on the form...

  • MaxiTB (unregistered)

    Why do all those people even bother to write something like that, even tho there asp.net came out of the box with injection filters from the very start?

  • Sauron (unregistered)

    I like how they named the function "PreventSQLInjection".

    The very idea that just calling a function to check actively for an attack is a WTF in itself.

    It is not just a wrong implementation, it is a wrong paradigm. The code should first of all have on a structural, passive resilience (for the systematic use of prepared statements for SQL queries is good because it is a structural resilience)

  • (nodebb)

    AnD(We're(off)),(Or(nOT))?

    Assumptions about hackers being consistent case-wise and ignorant of the fact that spaces aren't crucial to getting a query through are silly.

    Is it really that hard to go the easy way of just escaping any (direct or indirect) outside input before putting it into a local API?

  • Barry Margolin (github)

    it breaks pretty much any query you pass to it.

    But that does prevent SQL injection. That's all it's supposed to do.

  • (nodebb) in reply to Barry Margolin

    "yes and no" (but thanks to the function it's a resounding "yes" ;-)

  • DeathBeNotLoud (unregistered)

    The function itself is fine, it's just misnamed. It should be called PreventAlmostAnyQueryFromProperlyExecuting or DontCallThis.

  • TS (unregistered) in reply to Dragnslcr

    ... and then trim it. I don't have a c# compiler to hand to check, but surely if passed any string with a space in it, this will return an empty string.

  • Prime Mover (unregistered)

    Looks like eKRÉTA is in danger of becoming exKRÉTA.

  • Sou Eu (unregistered) in reply to TS

    Trim will remove any whitespace from the end of the string (there is another form to trim from the front, and a third form which trims from the front and back). The resulting string might have whitespace.

  • (nodebb)

    The best part? According to someone who claims to have downloaded the full source code, this function is never called.

    Correct. That is the best part.

    It's a good thing they put the "dangerous" keywords in upper cased AnD lowercase so there is Not a way to fool the code oR hack it.

  • Klimax (unregistered)

    Correction for Remy Porter: GDPR is going to be applied by Hungarian Data Protection agency (assuming company is Hungarian). Not that it is going to prevent morons from screaming EU anyway...

  • JS (unregistered)

    Little Bobby Tables? Anyone..?

  • LCrawford (unregistered) in reply to Dragnslcr

    it breaks pretty much any query you pass to it.

    It's likely meant to filter 'perceived badness' out of each query parameter, not the entire query. Of course it would still fail at that task as written.

  • (nodebb) in reply to Dragnslcr

    I don't think it's supposed to be called on queries, but on the input values used to build the query. But yeah, if you input "hello world", it will only insert "hello". And if your name is O'Neil, well, too bad, it's now ONeil.

  • Greg (unregistered)

    Why do you say cleartext contains no spaces? Only leading and trailing spaces are trimmed, but there can still be spaces within the resulting value.

  • Greg (unregistered) in reply to Dragnslcr

    This!

  • xtal256 (unregistered) in reply to Greg

    It substrings up to the first space, then trims. So the string " this is a string" will return "this".

  • (nodebb) in reply to tom103

    Yeah, I thought of that after I had posted, but my way would be a bigger WTF, so I'm gonna stick with that.

  • Chris Travers (unregistered)

    If course it is never called. What happens if you try to pass the name "Amanda" through it? If you cannot pass names through it, what good is it?

  • Daryl (unregistered)

    Around the time that SQL injections became popular, one of my webdevs was a bit green and when I reviewed his code it was string concatentation everywhere, wide open for this type of attack. I told him about SQL injection, and demonstrated that it worked on our live website, and instructed him to fix it by using parameters for all his queries instead of string concatenation.

    When I checked back a few weeks later, I found that he had changed all instances from this sort of thing:

    sql = "SELECT id FROM table WHERE code=" & code

    to this sort of thing

    Function assembleSQL(ACode) assembleSQL="SELECT id FROM TABLE WHERE code=" & ACode end Function

    sql = assembleSQL(code)

    He'd misunderstood everything about everything and presumed that somehow passing an argument to a function was the same as using a parameter and therefore somehow stopped SQL injection.

    I still don't understand how he did all of those changes across hundreds of instances throughout the codebase without googling or asking me for clarification on what he was doing.

  • issrar78 (github)

    Doesn't IndexOf return the location of the first occurrence of the string that you're searching for? So wouldn't this method cut out everything after the first space in the string?

    No surprise the function is never called; it breaks pretty much any query you provide to it. check out this https://apkofdark.blogspot.com/2022/11/vip-nobita-ff-apk-latest-version-v19312.html

  • Steve (unregistered)

    https://dotnetfiddle.net/BLY0t6 for anyone who wants to play with it, it does keep the first part of the string until the space, so it isn't returning empty strings (unless it starts with a space)

  • Kleyguerth (github) in reply to Daryl

    Same thing happened to me, but the roles were inverted. I was the green dev who found out the app was vulnerable to SQL injection, showed it to the seniors and proposed using prepared statements. Their solution was to do exactly what today's article code tries to do: manually remove SQL keywords. The best part was that it broke one of the admin users password, because it contained the word "from".

  • Jay (unregistered)

    That list of tags does not include all the key words in SQL, not even the common ones. What about ";", "SELECT * FROM", "DROP TABLE", or "INSERT INTO"? Plenty of damage could be done with that if the goal is to overload the system or steal data. It's also missing the ">" operator which means "<" can be overridden by just reversing the arguments.

  • (nodebb) in reply to Kleyguerth

    So the password wasn't hashed by the time it headed off to the data layer? Which means it probably wasn't hashed at all, eh? Hoo boy.

  • (nodebb) in reply to Chris Travers

    Oh don't worry, it won't replace the "and" in "Amanda" because the disallowed keyword " and " is surrounded by spaces! Good to go!

  • VeraLibera (unregistered)

    So, how would this function respond to "something a and nd another"? If it were called, I mean.

  • Pavilon (unregistered)

    In the next article at Telex an independent expert and a former employee are interviewed and turns out that the published parts are legacy code from a previous software and dead long ago. Also all data found there such as passwords look like test data. The incident itself started by simple phishing. I did not see any report on stolen personal data at telex since then. I am pretty sure they would be happy to report it.

  • Jeremy (unregistered)

    Did They Find Out You Don't Need Spaces In Your Query?

Leave a comment on “Hungry For an Education”

Log In or post as a guest

Replying to comment #:

« Return to Article