• faoileag (unregistered)
    Dan Adams-Jascobson:
    $rs = new RecordSet("SELECT * FROM moduleData WHERE moduleID = '".$moduleID."' ORDER BY displayOrder ASC");
    Since that code just selects a single record by its primary key, the only thing wrong with it is the redundant ORDER BY clause.
    $moduleID = '0; DROP TABLE moduleData;'
    CAPTCHA: ideo - that code snippet gave me an ideo!
  • (cs)

    Maybe $moduleID is sanitized earlier ;)

  • Jörg (unregistered)

    Nice Lovecraft-style there!

  • Franky (unregistered)

    well, at least he now knows what to do for the next 20 years ... clean up the insanity piece by piece.

  • Yazeran (unregistered) in reply to Iwamori
    Iwamori:
    Maybe $moduleID is sanitized earlier ;)
    Based on the SQL insanity, I seriously doubt that would be the case, byt who knows; more strange things have happened....

    Yazeran.

    Plan: To go to Mars one day with a hammer.

  • Tim (unregistered)

    Hmm, TRWTF is the actual story itself :O

    Well done guys - this is what thedailywtf should be like every day :-)

  • X (unregistered)

    That... that is even worse than our PHP...

  • foo (unregistered)

    Hey that sounds like I know the code, from my first job. The guys there had a tendency to write similar stuff.

  • F (unregistered)

    "Shed a year"? I could usefully shed several. Will re-reading the article achieve that?

  • ezacharyk (unregistered)

    "shed a year" - I just sucked one year of your life away. Be honest. How do you feel?

  • QJo (unregistered)

    I think that's quite neat code. It's clearly been thought through well and responsibly, and the fact that error handling is minimal and cursory suggests that the function works really, really well.

    Move along there, nothing to see.

  • shadowswiss (unregistered)

    And nobody even picked up on the sql injection issue.....

  • Martijn (unregistered) in reply to Iwamori
    Iwamori:
    Maybe $moduleID is sanitized earlier ;)
    Maybe, or maybe not. You can't really tell, and that's why SQL query building through concatenation is bad. Always, always, always use parameterized queries.

    THIS is the only thing wrong with the query. Who cares about a redundant ORDER BY?

  • Martijn (unregistered) in reply to shadowswiss
    shadowswiss:
    And nobody even picked up on the sql injection issue.....
    Yeah, it took all the way until the very first comment for someone to pick up on that.
  • amomynous (unregistered) in reply to shadowswiss
    shadowswiss:
    And nobody even picked up on the sql injection issue.....
    Probably nobody was bothered to point out the obvious. In such a monstrosity, SQL injection is not a bug or even a feature, it is a requirement.
  • Al (unregistered)

    Sure, I'll shear a yed.

  • Markus (unregistered) in reply to amomynous
    amomynous:
    shadowswiss:
    And nobody even picked up on the sql injection issue.....
    Probably nobody was bothered to point out the obvious. In such a monstrosity, SQL injection is not a bug or even a feature, it is a requirement.
    Yeah, SQL injection is how they plan to add new features. Just append

    '; ...New SQL Statement for New Feature Here... ;'

    to one of the results of one of those calls. Preferably to one that ends the query.

  • (cs) in reply to Al
    Al:
    Sure, I'll shear a yed.
    It's yed shaving day already? Dang, it seems like it happens earlier every year.
  • Dave (unregistered) in reply to shadowswiss
    shadowswiss:
    And nobody even picked up on the sql injection issue.....

    Because there isn't one. None of the underlying PHP mysql(i)_query functions will allow that second query to be executed.

  • cyborg (unregistered) in reply to RichP
    RichP:
    Al:
    Sure, I'll shear a yed.
    It's yed shaving day already? Dang, it seems like it happens earlier every year.

    It is because yed shearing follows the Julian Calendar.

  • Anonymous (unregistered) in reply to faoileag
    faoileag:
    Dan Adams-Jascobson:
    $rs = new RecordSet("SELECT * FROM moduleData WHERE moduleID = '".$moduleID."' ORDER BY displayOrder ASC");
    Since that code just selects a single record by its primary key, the only thing wrong with it is the redundant ORDER BY clause.
    $moduleID = '0; DROP TABLE moduleData;'
    $moduleID = "0'; DROP TABLE moduleData; --"

    Kind of FTFY, but it probably only executes one query so your plan is busted.

    Really FTFY:

    $moduleID = "' OR 'a' = 'a"
  • Enrique (unregistered)

    I've said it before: PHP is just a DSL for writing SQL injection entry points.

  • Bananafish (unregistered)

    Not only do I think Simon wrote this, but I also think I know which particular Simon it was.

    We've got tons of this in legacy PHP/SQL projects that a former employee, Simon, wrote while he was here.

    Coincidence?

  • JT (unregistered)

    I've always felt eval() was a solution in search of a problem.

  • (cs)

    SELECT * in a production environment.

    There's your sign.

  • Mike Dimmick (unregistered)

    I realise that this has many other problems, but I can't see why you think the ORDER BY is redundant. Databases are under no obligation to produce results in any particular order unless ORDER BY is specified. The order that results come out can and will change depending on the available indexes, and the exact query plan that the server chooses based on the statistics available. Adding or deleting data can cause the order to change. Adding or deleting indexes could cause the order to change even if you weren't intentionally using those indexes.

    The application might not actually care in which order they come out, but that isn't explained in the article. That the column is called 'displayOrder' is a big clue that it actually does matter.

  • Anonymii (unregistered) in reply to Mike Dimmick
    Mike Dimmick:
    I realise that this has many other problems, but I can't see why you think the ORDER BY is redundant. Databases are under no obligation to produce results in any particular order unless ORDER BY is specified. The order that results come out can and will change depending on the available indexes, and the exact query plan that the server chooses based on the statistics available. Adding or deleting data can cause the order to change. Adding or deleting indexes could cause the order to change even if you weren't intentionally using those indexes.

    The application might not actually care in which order they come out, but that isn't explained in the article. That the column is called 'displayOrder' is a big clue that it actually does matter.

    The point is, since moduleID is the primary key (and probably the unique key), there will only ever be one record returned from that query, thus an ORDER BY clause is redundant because you can't really order a single record.

    TRWTF is indeed that no fucks were given to the blatant SQL injection hole.

  • LyonesGamer (unregistered) in reply to Mike Dimmick

    I think the reason why the ORDER BY was considered redundant is because it's looking it up by moduleId which is (hopefully, if it's called an ID) unique. So it should only return one result. So an ORDER BY would be meaningless.

  • AnonyMark (unregistered) in reply to Mike Dimmick
    Mike Dimmick:
    I can't see why you think the ORDER BY is redundant.

    Because the article stated that you're selecting by primary key, hence you will never get more than one item back. Sorting lists with less than two items is a bit silly.

  • (cs) in reply to Mike Dimmick
    Mike Dimmick:
    I realise that this has many other problems, but I can't see why you think the ORDER BY is redundant. Databases are under no obligation to produce results in any particular order unless ORDER BY is specified. The order that results come out can and will change depending on the available indexes, and the exact query plan that the server chooses based on the statistics available. Adding or deleting data can cause the order to change. Adding or deleting indexes could cause the order to change even if you weren't intentionally using those indexes.

    The application might not actually care in which order they come out, but that isn't explained in the article. That the column is called 'displayOrder' is a big clue that it actually does matter.

    article:
    Since that code just selects a single record by its primary key
    Seems redundant to tell the database to sort a list of one record...
  • (cs)

    i have seen worse code than this so i am questioning if this merits to be a wtf.

  • iytourotiurnioturpoi (unregistered)

    So why all the concern about SQL injection? If it's a system where everyone is trusted, is a responsible adult, knows what they're doing, and (possibly in some people's eyes the most important thing) has easier ways to destroy the system - then why be concerned?

    Or does everyone have a belt and a pair of braces on their onesie just in case?

    CAPTCHA: suscipit - not moving because of the suspicion that there may be a pit somewhere

  • (cs)

    Ia! Ia! SQLthulhu Fhtagn!

  • (cs) in reply to Nagesh
    Nagesh:
    i have seen worse code than this so i am questioning if this merits to be a wtf.

    Dear co-worker of Nagesh: please submit his code to this site.

  • (cs) in reply to iytourotiurnioturpoi
    iytourotiurnioturpoi:
    So why all the concern about SQL injection? If it's a system where everyone is trusted, is a responsible adult, knows what they're doing, and (possibly in some people's eyes the most important thing) has easier ways to destroy the system - then why be concerned?

    Or does everyone have a belt and a pair of braces on their onesie just in case?

    CAPTCHA: suscipit - not moving because of the suspicion that there may be a pit somewhere

    That's all fine and good until a responsible, trusted, but non-technical adult works on a record for Bobby Tables.

  • Niklaus Wirth (unregistered)

    That site wouldn't exist if everyone would use PROPER programming language. All I see are PHP, C++, C# WTFs here, there are none Pascal or Delphi ones. Why? Obviously, because Pascal is a superior language.

    And before you'll try to say it's because it's not popular, well many businesses both big and small uses Delphi or even legacy Turbo Pascal apps (for accounting, HR and similar tasks).

    Hell, if you are game developer, you may be using program written in Delphi and not even realize it. GraphicsGale, popular pixelart editor is written in Delphi.

    So if Pascal wouldn't be superior language, some Pascal/Delphi WTF would crop up on this site sooner or later, yet it didn't.

  • (cs)

    It's better if you read it backwards (bottom-to-top). That way:

    • You'll go mad from the unparenthesized nested ternaries first.

    • Then, the PHP-code-embedded-in-query-construction will hurt less.

    • By the time you've parsed enough to see what the query actually is, you'll be prepared to simply laugh maniacally at the SELECT * FROM That Which Is Not A Table.

    • After all that, when you reach the beginning, the mundane WTFery of a maybe-SQL-injection will be a breath of fresh air -- the beginning of the road back to sanity.

  • (cs) in reply to iytourotiurnioturpoi
    iytourotiurnioturpoi:
    If it's a system where everyone is trusted, is a responsible adult, knows what they're doing...

    Your assumption is incorrect. Exhibit 1: The code in the article.

  • amomynous (unregistered) in reply to Niklaus Wirth
    Niklaus Wirth:
    That site wouldn't exist if everyone would use PROPER programming language. All I see are PHP, C++, C# WTFs here, there are none Pascal or Delphi ones. Why? Obviously, because Pascal is a superior language.

    And before you'll try to say it's because it's not popular, well many businesses both big and small uses Delphi or even legacy Turbo Pascal apps (for accounting, HR and similar tasks).

    Hell, if you are game developer, you may be using program written in Delphi and not even realize it. GraphicsGale, popular pixelart editor is written in Delphi.

    So if Pascal wouldn't be superior language, some Pascal/Delphi WTF would crop up on this site sooner or later, yet it didn't.

    Did Wirth or some Pascal die-hard advocate came out with some Pascal proof of superiority like this? If so, please, a link is welcome.

    Note: my first programming language was Pascal, but I haven't touched it in like 10 years. It was cool.

  • Masaaki (unregistered)

    Holy shit, it's a Cthulu-summoning Rube Goldberg machine of PHP and SQL.

    I'm gonna go cry in a corner now.

  • Norman Diamond (unregistered) in reply to cyborg
    cyborg:
    RichP:
    Al:
    Sure, I'll shear a yed.
    It's yed shaving day already? Dang, it seems like it happens earlier every year.
    It is because yed shearing follows the Julian Calendar.
    You guys are on a tear.
  • Norman Diamond (unregistered) in reply to amomynous
    amomynous:
    Niklaus Wirth:
    That site wouldn't exist if everyone would use PROPER programming language. All I see are PHP, C++, C# WTFs here, there are none Pascal or Delphi ones. Why? Obviously, because Pascal is a superior language.

    And before you'll try to say it's because it's not popular, well many businesses both big and small uses Delphi or even legacy Turbo Pascal apps (for accounting, HR and similar tasks).

    Hell, if you are game developer, you may be using program written in Delphi and not even realize it. GraphicsGale, popular pixelart editor is written in Delphi.

    So if Pascal wouldn't be superior language, some Pascal/Delphi WTF would crop up on this site sooner or later, yet it didn't.

    Did Wirth or some Pascal die-hard advocate came out with some Pascal proof of superiority like this? If so, please, a link is welcome.
    No. Wirth invented Pascal, not Turbo Pascal. Pascal was useful for teaching. Turbo Pascal was useful for programming PCs. The relationship between Pascal and Turbo Pascal was akin to the relationship between Java and JavaScript.

    amomynous:
    Note: my first programming language was Pascal, but I haven't touched it in like 10 years. It was cool.
    I suppose it was around my 10th programming language or so, and I haven't touched it in 26 years. I had to laugh when people who expressed such hatred for it happily copied features from it.
  • (cs) in reply to iytourotiurnioturpoi
    iytourotiurnioturpoi:
    So why all the concern about SQL injection? If it's a system where everyone is trusted, is a responsible adult, knows what they're doing, and (possibly in some people's eyes the most important thing) has easier ways to destroy the system - then why be concerned?

    At the very least, someday someone is going to enter John "The Man" O'Mally and get some weird SQL error instead of it working. In this case, it looks like the error would disappear, leading to silently disappearing entries and a likely accounting with some very unhappy people who lost a bunch of data attached to some valuable accounts.

  • Cheong (unregistered)

    This is one of the times where anything except complete rewrite won't cut it.

  • Dave (unregistered)

    Did anyone else nearly pass out from sheer boredom from reading this?

  • Thomas (unregistered) in reply to Dave
    Dave:
    Did anyone else nearly pass out from sheer boredom from reading this?

    Are you serious? You must be at the wrong site. This was one of the most well written stories I've seen here. Excellent!

  • Hannes (unregistered) in reply to Thomas
    Thomas:
    Dave:
    Did anyone else nearly pass out from sheer boredom from reading this?

    Are you serious? You must be at the wrong site. This was one of the most well written stories I've seen here. Excellent!

    Better than Hanzo? You've got to be kidding me!

  • Blackdaemon (unregistered) in reply to AnonyMark

    Sorry, but no. Article stated that they originally assumed that what the SQL does is selecting by primary key. But that was obviously wrong assumption to start with. Unless you know the table and keys definitions, you are not allowed to assume anything like that just by looking at the select statement.

  • venio (unregistered) in reply to Blackdaemon
    Blackdaemon:
    Sorry, but no. Article stated that they originally assumed that what the SQL does is selecting by primary key. But that was obviously wrong assumption to start with. Unless you know the table and keys definitions, you are not allowed to assume anything like that just by looking at the select statement.
    Because you naturally have more information on that subject than the submitter of the article?
  • ac (unregistered)

    It is interesting that everyone considers returning anything other than raw data stored in database using SQL as a WTF yet all web programming languages - PHP, ASP(.NET), Javascript, and JSP just the name a few - are all about embedding and mixing just about every possible technology one can lay their hands on and this is considered normal, acceptable, and even cool. NoSQL databases are all about running any programming language you can think of against unstructured data sometimes containing XML, HTML, or JSON documents and returning back the something that could be even a reformatted web page and this is a great new idea that increases performance. Do the same in SQL and you have a WTF. I am not saying that returning PHP form database is a great idea but what is the difference between that and PHP (or JSP or ASP.NET) using embedded SQL (directly or via some fancy ORM) and generating HTML with embedded Javascript? Isn't the way modern web development works all a WTF on top of WTF?

Leave a comment on “The Database Gazes Also Into You”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article