• mike5 (unregistered)

    Oh, stop being picky!

  • Keloran (unregistered)

    boot.ini ... this must be a very odd SQL vuln

  • whatever (unregistered)

    Provide a link, and I'll wager they'll quickly change their minds.

  • True that (unregistered)

    That particular client deserves everything they get..

    Captcha: erat - you thought live vermin were bad, what about virtual ones?

  • a programmer (unregistered)

    that is probeily the frist time they ever faced sql injection problems

  • Slicerwizard (unregistered)

    Typical quality prose from Alex.

  • (cs) in reply to whatever
    whatever:
    Provide a link, and I'll wager they'll quickly change their minds.
    Alex won't, I'm sure, even if the submitter gave him the url. Which is a good thing :)
  • (cs)

    It looks like they put their best VB programmer on the job.

  • Joe Hacker (unregistered)

    hmm, I guess all we "superhackers" needed to pay better attention in Math class. Apparently, through the miracle of modern math, 2 != 2, but I guess I'm too stupid to understand this kind of wonder.

    Captcha: minim. What a pity these guys don't even have a minim of programming ability.

  • Bill P. Godfrey (unregistered)

    They should have used an embedded system, because without a file system, there's nowhere to store a database.

  • (cs) in reply to Bill P. Godfrey
    Bill P. Godfrey:
    They should have used an embedded system, because without a file system, there's nowhere to store a database.

    Sure there is. XML in a null terminated string created at design-time.

  • Befuddled (unregistered)

    Easy one to fix.

    After installing, just set up a cron job that sabotages the application once a week with a semi-random SQL injection.

    It will infuriate the morons that bought it and cost the cowboys that sold it to them a fortune in support.

  • Paul (unregistered)

    Won't the first bit protect against SQL injections? So, it is actually safe now?

    OK, the second bit (all the replaces) is dumb, but converting ' to '' is all that is required isn't it (even though it is being done in a lame way in this case)?

    AIUI SQL injection attacks are thing like entering: ' OR 23!=46-- in a username box. By quoting the ' character you've prevented the attack

    Without the quoting you get SQL queries like: select * from users where username='' OR 23!=46--' so, it will return everything. With the quoting you get select * from users where username=''' OR 23!=46--' so it will return nothing (unless there is a user called "' OR 23!=46--")

    Or am I missing something?

  • anonym (unregistered)

    select * from thistable where 1=1 ----

    select * from thistable where 1=1 --

    wohoo!

  • Patrick (unregistered)

    They don't even need the '='. Just a simple '1' would do it.

  • Nibh (unregistered) in reply to anonym

    "alter table" seems to be in there twice. Perhaps because it's twice as dangerous? This code is way above my head.

  • Pegasus (unregistered)

    OMG, i remember such code being in production at the largest webshop around here ... Talk about lame ;)

  • Probeily Some Guy (unregistered) in reply to Nibh
    Nibh:
    "alter table" seems to be in there twice
    Stop beeing picky.
  • Seminymous Coward (unregistered)

    If I understand correctly, and the reviewer is the corporation's internal IT department, the department head is feckless. He should flat-out refuse to install this. If he doesn't have that authority, he needs to get it to do his job effectively, so he'd best get to convincing whoever gives out that authority.

  • grzlbrmft (unregistered)
    ...they told us were beeing too picky, and had us install the application anyway.
    Uhm... the answer to such requests is "No". Otherwise that's the real WTF.
  • Rob (unregistered)

    Am I the only one who noticed the name of the function is "FQ"?

  • SilentRunner (unregistered)

    It would be nice if the person who posts a WTF concerning code would explain what is wrong with the code. It is not always obvious, especially if it's in a language we're unfamiliar with.

  • Tim (unregistered) in reply to Paul
    Paul:
    Won't the first bit protect against SQL injections? So, it is actually safe now?

    OK, the second bit (all the replaces) is dumb, but converting ' to '' is all that is required isn't it (even though it is being done in a lame way in this case)?

    Or am I missing something?

    You're missing something:

    ''; DROP TABLE users; --

    Source: http://unixwiz.net/techtips/sql-injection.html

    Also, captcha quoters: You're not funny. You're tedious. Tedious and boring.

  • (cs)

    Only a matter of time now before someone posts a link a certain webcomic, which will no doubt be followed by a thousand people groaning...

  • Probeily Some Guy (unregistered) in reply to Seminymous Coward
    Seminymous Coward:
    ... the department head is feckless.
    Congratulations! You have won 4 internets for integrating the word "feckless" into your post.
  • aristos_achaion (unregistered) in reply to galgorah
    galgorah:
    whatever:
    Provide a link, and I'll wager they'll quickly change their minds.
    Alex won't, I'm sure, even if the submitter gave him the url. Which is a good thing :)

    Oh, I don't know. I tend to find that submitting an obviously-not-sanitized form saying "I could delete your database right now; you're lucky I'm a scrupulous person" tends to get people's attention.

  • or 2=2 and DROP Johnny Tables (unregistered) in reply to SilentRunner

    The problem with the code is it is trying to catch strings that could do bad things to the database.

    Imagine a field on the webform that asks for your name.

    You enter: John Doe.
    The app converts that to something like "select record from customers where name='John Doe';

    No problem.

    Now assume for the name you enter 1=1; DROP TABLE CUSTOMERS; --

    Now the app converts that to: select record from customers where 1=1; DROP TABLE Customers; -- this lass double dash ensures anything after gets treated as a comment.

    You've just managed to drop the customer table from the database.

    Obviously their approach can't catch every possible combination.

    My above example might be caught, but that's fine. I can then try 2=2; SELECT * from customers; --

    Now I can get a full list of all their customers.

    CAPTCHA: Saluto - I saluto the geniuses who thought up this solution.

  • (cs)

    Also notice the script tag/javascript filtering at the end - stopping SQLi and XSS at the same time!

  • bd (unregistered)

    It's also missing "Robert); DROP TABLE Students;--?"

    (http://xkcd.com/327/ for the uninitiated)

  • NoAstronomer (unregistered)

    and of course in a years time some poor sap of a developer will be trying to use a query that does actually need a UNION and will have no idea why it doesn't work.

  • aaawww (unregistered) in reply to Paul

    @paul: there is the problem of indirect sql injection. once you got a ' on the server, if the server trusts its own data to build the joins, then you are escaping the joined query

    this is protected not trusting even the server stored data, but why in hell isn't everybody using named parameters query, for god sake? the database KNOW how to escape it, and it has the added benefit of providing the same statement for different inputs, so the database may actually do its thing and cache the query path.

  • Tim (unregistered) in reply to Paul
    Paul:
    Won't the first bit protect against SQL injections? So, it is actually safe now?
    I believe you are correct. I think mysql can also use backquotes to escape a quote character with in a string, so there might be an additional vuln if the user entered something like

    ' or 1=1

    because this code would convert it into ''

  • cardkey (unregistered) in reply to DOA
    DOA:
    Only a matter of time now before someone posts a link a certain webcomic, which will no doubt be followed by a thousand people groaning...

    Ask and you shall receive

    http://xkcd.com/331/

    Also TRWTF is this site.

  • DeepThought (unregistered) in reply to SilentRunner
    SilentRunner:
    It would be nice if the person who posts a WTF concerning code would explain what is wrong with the code. It is not always obvious, especially if it's in a language we're unfamiliar with.

    I'm not overly familiar with the language used in this case either, but the failure of the code is fairly obvious to anyone experienced with SQL injection issues. The code does make an attempt to sanitize the query by doubling up the single quote characters, but depending on the DBMS this may not be the only way of injecting arbitrary SQL into a request. Also, it still leaves the code vulnerable to cross-site scripting attacks by failing to catch for all potential of attack vectors.

    Of course, it's almost impossible to catch all known and future attack vectors using a technique such as the one demonstrated in the code as the variations could be endless (example: "language = javascript" would not be caught). It would be far better to use parametrized SQL statements, stored procedures, and/or to HTML encode the incoming data before persisting it in the DB thus making it safe to display in a browser.

  • dude (unregistered) in reply to cardkey
    cardkey:
    DOA:
    Only a matter of time now before someone posts a link a certain webcomic, which will no doubt be followed by a thousand people groaning...

    Ask and you shall receive

    http://xkcd.com/331/

    Also TRWTF is this site.

    Groan

  • Anonymously Yours (unregistered)

    I'm guessing that since it's a customer of yours you're in the dubious position. While you're trying to get them to accept the discount programming they paid for was worthless, you can't outright refuse them without potentially losing them, and your objections may come off as trying to bolster your own sales department.

    I think your best out in that situation would be to install the software and then give your customer instructions to follow that would make them crap themselves. You could either have them create a new administrator account or have them to cripple their database, and in either case they'd do so with copy-and-paste operations between your email and the app. If you went with the crippling route, this would preferably by with an UPDATE statement that changes some system value that can easily be reset back to what it should be with another UPDATE. You know, nothing that would require a full DB restore.

  • Quirkafleeg (unregistered) in reply to Tim
    Tim:
    Paul:
    Won't the first bit protect against SQL injections? So, it is actually safe now?

    OK, the second bit (all the replaces) is dumb, but converting ' to '' is all that is required isn't it (even though it is being done in a lame way in this case)?

    Or am I missing something?

    You're missing something:

    ''; DROP TABLE users; --

    Source: http://unixwiz.net/techtips/sql-injection.html

    I would have thought that handling \ would be required, given what's going to be parsing the string. Or perhaps the author of that alleged injection prevention code failed to consider that because the language which is used to invoke the query parser doesn't use \ for escaping? (WTF right there.)

    Is there no suitable string escaping (preferably shell-style) function in the language or a relevant library for that language?

  • (cs) in reply to whatever
    whatever:
    Provide a link, and I'll wager they'll quickly change their minds.
    QFT. ;D
  • Paul (unregistered) in reply to Tim
    Tim:
    You're missing something:

    ''; DROP TABLE users; --

    ???

    '' is just '' in SQL.

    So you will end up with

    ''''; DROP TABLE users; --

    which can't be used for SQL injection.

    or are you using a SQL-like database which treats '' as a metacharacter?

    Source: http://unixwiz.net/techtips/sql-injection.html
    Ah, that only works with MySQL. SQL compliant databases won't suffer from that weakness.
  • Tim (unregistered) in reply to Quirkafleeg
    Quirkafleeg:
    Is there no suitable string escaping (preferably shell-style) function in the language or a relevant library for that language?
    It's nothing to do with quoting in the programming language it was written in, only to do with the SQL syntax used by the database. If the user entered \' into an input field, you would want to convert it into \'' if you were using mssql but into \\'' (or \\\') if you were using mysql.
  • Paul (unregistered) in reply to Tim
    Tim:
    I believe you are correct. I think mysql can also use backquotes to escape a quote character with in a string, so there might be an additional vuln if the user entered something like

    ' or 1=1

    because this code would convert it into ''

    OK, but how do we know they're using MySQL?

    If they're using an SQL compliant DB, then quoting '' characters would be a bug, not a security fix, so, without knowing what DB they're using, their fix might be safe (but stupid) or not safe at all.

  • Seth (unregistered) in reply to grzlbrmft
    grzlbrmft:
    ...they told us were beeing too picky, and had us install the application anyway.
    Uhm... the answer to such requests is "No". Otherwise that's the real WTF.
    Or at least "First sign this statement that you acknowledge our report that the code is vulnerable and installing it will cause security problems."
  • Coincoin (unregistered)

    It's official, 331 == Frist!11

  • aristos_achaion (unregistered) in reply to Paul
    Paul:
    Tim:
    I believe you are correct. I think mysql can also use backquotes to escape a quote character with in a string, so there might be an additional vuln if the user entered something like

    ' or 1=1

    because this code would convert it into ''

    OK, but how do we know they're using MySQL?

    If they're using an SQL compliant DB, then quoting '' characters would be a bug, not a security fix, so, without knowing what DB they're using, their fix might be safe (but stupid) or not safe at all.

    Or they could sidestep the whole issue by using a real fix, like parametrized queries.

  • Helix (unregistered) in reply to SilentRunner
    SilentRunner:
    It would be nice if the person who posts a WTF concerning code would explain what is wrong with the code. It is not always obvious, especially if it's in a language we're unfamiliar with.

    This is typical for an embedded programmer without file system experience

  • Quirkafleeg (unregistered) in reply to Tim
    Tim:
    Quirkafleeg:
    Is there no suitable string escaping (preferably shell-style) function in the language or a relevant library for that language?
    It's nothing to do with quoting in the programming language it was written in, only to do with the SQL syntax used by the database.
    It is to do with that language. Where else are you (sensibly¹) going to do the quoting?
    If the user entered \' into an input field, you would want to convert it into \'' if you were using mssql but into \\'' (or \\\') if you were using mysql.
    You're right in that what quoting is required is dependent on the target, though.

    ¹ Javascript. Just Say NoScript.

  • Probeily Some Guy (unregistered) in reply to Rob
    Rob:
    Am I the only one who noticed the name of the function is "FQ"?
    Nope, you aren't. My theory is that it had a different name before they were asked to prevent SQL injections... they renamed it "FQ" as a passive-aggressive way of stickin' it to the man for thinking that the code wasn't already perfect the first time.
  • Griphon (unregistered)

    Why doesn't anyone address the security side of it. Minimum permissions, grant the select, insert, delete, update ONLY as needed, but don't grant DBO permissions to any user accounts. This way the outside vendor can deliver the crappy code, and you can install it anyway, but you're less concerned about damage. Still, using views, stored procedures and other good practices should be the norm, but the reality is that you can't control everything, so control what you can.

  • the_manager (unregistered) in reply to aristos_achaion
    aristos_achaion:
    Paul:
    Tim:
    I believe you are correct. I think mysql can also use backquotes to escape a quote character with in a string, so there might be an additional vuln if the user entered something like

    ' or 1=1

    because this code would convert it into ''

    OK, but how do we know they're using MySQL?

    If they're using an SQL compliant DB, then quoting '' characters would be a bug, not a security fix, so, without knowing what DB they're using, their fix might be safe (but stupid) or not safe at all.

    Or they could sidestep the whole issue by using a real fix, like parametrized queries.

    That costs too much money. Any real manager knows that.

  • RBoy (unregistered) in reply to Tim
    Tim:
    Paul:
    Won't the first bit protect against SQL injections? So, it is actually safe now?

    OK, the second bit (all the replaces) is dumb, but converting ' to '' is all that is required isn't it (even though it is being done in a lame way in this case)?

    Or am I missing something?

    You're missing something:

    ''; DROP TABLE users; --

    Source: http://unixwiz.net/techtips/sql-injection.html

    Also, captcha quoters: You're not funny. You're tedious. Tedious and boring.

    Sorry, didn't mean to quis you off.

Leave a comment on “Injection Proof'd”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article