• IN-HOUSE-CHAMP (unregistered) in reply to ObiWayneKenobi
    ObiWayneKenobi:
    eVil:
    Did you just mix Buffy with Lord of the Rings? You, sir, disgust me.

    This. We need to throw somebody in Mount Doom for this egregious offense to geekdom. Hell, it's an offense to God Himself.

    Somebody get me a pot and melt down some gold. We're giving this guy a golden crown to "reward" him for this "greatness".

    Can't wait to TORRENT the next season of GAME OF THRONES!

  • IN-HOUSE-CHAMP (unregistered) in reply to Lorne Kates
    Lorne Kates:
    eVil:
    Tipa:
    Angel, not Buffy......

    If I recall correctly, the universe in which both shows reside is known as the Buffyverse. Presumably on the basis that that was the popular show that people liked, and Angel was the also-ran spin-off that people mostly ignored.

    Either way, mixing it with LOTR makes you a bad, bad person.

    The shame's on you. You do know that there were LOTR books before the movie, right? Just like a typical movie bandwagon-hopped. Peter Jackson directed the movies, but Joss Whedon wrote the original books.

    Good trolling Lorne, about 50 nerds are going to bite.

  • Just a guest (unregistered) in reply to faoileag
    "ALTER TABLE select RENAME update;" should return TRUE if fed into ValidateSQLText().
    Argh... Thats about what I ment to say -- but somehow I didn't.

    Although ALTER is not in the whitelist of commands it gets executed antway, as one of its arguments matches a word in the whitelist.

    In other words: why did the origional programmer match against all words in the string, and not just the first ...

  • (cs) in reply to Smouch
    Smouch:
    Seeing as he recognized the total uselessness of the validator function, why did he add the execute keyword at all?

    Why not comment out the code and replace it with

    return true;

    Because that's not using a function that has just "worked" for so long.

    When you play by the book and don't just rip things out when they've been in production, you're seen as more as a team player by the PHB. The more that you're willing to sacrifice your morals for what the people above want, the more money you're likely to make.

  • (cs)
    the DBA Joss:
    The codebase simply won’t support it. If your query requires anything other than SELECT, INSERT, UPDATE, OR DELETE, it fails.
    If your query contains any INSERT, UPDATE or DELETE, chances are high it is not a query but a DML-statement.

    Also grammar-WTF!

  • warlaan (unregistered) in reply to faoileag

    We store scripts that apply the necessary changes from one version to the next, but we don't store a complete creation script for the whole database schema.

    In theory that should be sufficient to tie the schema to the code version, but as our deployment tools do not automatically apply scripts to the database there are enough opportunities for human error.

    This is actually not the main reason why we don't use stored procedures - the main reason is that it was a company guideline not to use them. This is just the reason why we never felt much urge to break that rule.

  • (cs) in reply to Just a guest
    Just a guest:
    In other words: why did the origional programmer match against all words in the string, and not just the first ...

    Well, depending on the definition of "word", your queries/statements will look rather strange:

    SELECT * fRoM [^p^e^r^s^o^n^]
-- *w*i*l*l n*o*t w*o*r*k!

INSERT [^p^e^r^s^o^n^] (g,s,b)
-- *v*a*l*u*e*s(...) n*o*t a*l*l*o
w*e*d!
SELECT ('S' + 'U' + 'M' + 'M' + 'E' + 'R'),
 ('G' + 'L' + 'A' + 'U'),
 ('J' + 'U' + 'L' + 'Y 24 1981')

    and of course no 'BEGIN TRANSACTION' / 'COMMIT TRANSACTION', not even a 'go'!

  • (cs)

    valid:

    DROP TABLE Students; select 'SUCKS TO BE YOU!';
  • (cs)

    So if you can't write and use a stored procedure, use a trigger instead.

    Create some table and insert the parameterised records into it to generate the trigger.

    That gets you round the filter because INSERT is an approved word.

    In incidentally SQLServer allows MERGE which is a properly concurrent-safe "INSERT or UPDATE".

  • CodeBeater (unregistered)

    What if they wanted to select from a table called execute?

  • mrfr0g (unregistered)

    Obligatory XKCD: http://xkcd.com/327/

  • (cs) in reply to CodeBeater
    CodeBeater:
    What if they wanted to select from a table called execute?

    Create a view called "vExecuet" and select from that.

  • eVil (unregistered) in reply to mrfr0g
    mrfr0g:
    Obligatory XKCD: http://xkcd.com/327/

    You sir, disgust me also.

  • just stop it (unregistered) in reply to mrfr0g
    mrfr0g:
    Obligatory XKCD: http://xkcd.com/327/
    No. Is was obligatory the first 42 times the topic was related to SQL injection. Bringing it up from 43 on is just meaningless.
  • Dave (unregistered)

    I really thought this one was going to be that they keep all valid SQL statements in a table somewhere so that nobody can create an unapproved one.

    I am disappoint.

  • (cs)

    Could just change the function body to "Return True", but that might reject all user input as a potential SQL injection attack...

  • just stop it (unregistered) in reply to Dave
    Dave:
    I really thought this one was going to be that they keep all valid SQL statements in a table somewhere so that nobody can create an unapproved one.

    I am disappoint.

    That was the original approach. It was written as such:

    DECLARE @count int

    SELECT @count = COUNT(1) FROM validSQLStatements WHERE @statement LIKE Statement

    RETURN @count > 0

    validSQLStatements

    Statement

    %select% %insert% %update% %delete%

  • (cs) in reply to warmachine
    warmachine:
    Meh! He should have earned peer respect by revealing this function.

    But he is making program in Visual Basic. If you want respect, best code in Java.

  • (cs) in reply to chubertdev
    chubertdev:
    valid: 
    DROP TABLE Students; select 'SUCKS TO BE YOU!';

    Do you really think that schema access is provided to users? DBA is much smarter than normal guys.

  • (cs) in reply to IN-HOUSE-CHAMP
    IN-HOUSE-CHAMP:
    Lorne Kates:
    eVil:
    Tipa:
    Angel, not Buffy......

    If I recall correctly, the universe in which both shows reside is known as the Buffyverse. Presumably on the basis that that was the popular show that people liked, and Angel was the also-ran spin-off that people mostly ignored.

    Either way, mixing it with LOTR makes you a bad, bad person.

    The shame's on you. You do know that there were LOTR books before the movie, right? Just like a typical movie bandwagon-hopped. Peter Jackson directed the movies, but Joss Whedon wrote the original books.

    Good trolling Lorne, about 50 nerds are going to bite.

    Only on the absurd premise that Whedon wrote something original.
  • by by (unregistered) in reply to chubertdev
    chubertdev:
    valid: 
    DROP TABLE Students; select 'SUCKS TO BE YOU!';

    Msg 3701, Level 11, State 5, Line 1 Cannot drop the table 'students', because it does not exist or you do not have permission.

    SUCKS TO BE YOU!

  • Librarian (unregistered) in reply to QJo
    QJo:
    You'll have Commander Vimes on your tail at this rate.

    What do you mean you ain't got no tail? You're a code monkey, aren't you?

    Ook?!?

    Ook!

  • (cs) in reply to by by
    by by:
    chubertdev:
    valid: 
    DROP TABLE Students; select 'SUCKS TO BE YOU!';

    Msg 3701, Level 11, State 5, Line 1 Cannot drop the table 'students', because it does not exist or you do not have permission.

    SUCKS TO BE YOU!

    DELETE Students; -- SUCKS IF DELETE IS ALLOWED!
  • sagaciter (unregistered)

    It's very easy, actually:

    execute sproc_name_here_followed_by_args go select count(*) from some_known_table_name_here

  • sagaciter (unregistered) in reply to sagaciter
    sagaciter:
    It's very easy, actually:

    execute sproc_name_here_followed_by_args go select count(*) from some_known_table_name_here

    How stupid of me (that above); better yet:

    execute sproc_name_here @param1=@value1, ..., @paramN = @valueN; go; select 1;

    Something like that...

  • (cs) in reply to by by
    by by:
    chubertdev:
    valid: 
    DROP TABLE Students; select 'SUCKS TO BE YOU!';

    Msg 3701, Level 11, State 5, Line 1 Cannot drop the table 'students', because it does not exist or you do not have permission.

    SUCKS TO BE YOU!

    You are being waaaaaaaaaaaaaaaaaay too optimistic about the DBAs out there.

    They're just very lucky that it's not that exciting to see someone using the sa account to connect to a database, otherwise you would see a lot more DBA WTFs on this site.

  • (cs)

    He should have changed the function to do a regex. This matching pattern should have done it:

    /{s,d,ins,updat}*e{xe,le,le,rt}*{cu}*{te,ct}*/i

    (Oh yes, it only really matches an "e", but like the guy on the commercial said, "You're good!")

  • Spewin Coffee (unregistered)

    Why are the problem functions always "deep within the codebase"? The more, usually unnecessary, layers you add, the harder it is to debug. Can we please just fire all the "enterprise" developers already?

  • Valued Service (unregistered) in reply to faoileag
    faoileag:
    eVil:
    Now I have no idea what characters did who, where, or why in anything I've ever read, seen or heard.
    It's really simple.

    Once the mainframes ruled middle earth. But deep in the forests of mirkwood something was stirring. At first, it looked innocent and helpful. A pc, a "personal computer". Then Bill Gates came along and forged one ring to bind them all and ruled for aeons, erm, decades. But the ring was lost. Nothing happened for a while. Then a young hobbit named Woz discovered it, picked it up and used it occassionally but to no great harm. Nevertheless it attracted the attention of the great wizzard Obi Wan, who understand only to well the power of that ring. On Obi Wan's request, Steve passed the ring on to the young User who set out on an epic journey to destroy it. After a while, Obi Wan and User were joined by the elf Han Solo, the not so dwarf Chewee and some elf chick who can speak the R2-D2 language of beeps. Right before our beloved ending where User was to throw the ring into the fire, Jobs appeared and attempted to recover the ring to power iPhone's app store. Now it's up to user. Size with Jobs and face tyranny, or with Gates and face tyranny.

    FTFY

  • (cs) in reply to Librarian
    Librarian:
    QJo:
    You'll have Commander Vimes on your tail at this rate.

    What do you mean you ain't got no tail? You're a code monkey, aren't you?

    Ook?!?

    Ook!

    +1

  • (cs) in reply to Librarian
    Librarian:
    QJo:
    You'll have Commander Vimes on your tail at this rate.

    What do you mean you ain't got no tail? You're a code monkey, aren't you?

    Ook?!?

    Ook!

    "Excuse me, sir? Those students over there called you a monkey."

  • Anonymous (unregistered)

    Modifying the validation function could induce risk. I believe the least risky solution would be to add "-- select" to all execute statements, which makes them validate correctly

  • barf 4eva (unregistered)

    rewrite that mofo

  • Verisimilidude (unregistered) in reply to Nagesh
    Nagesh:
    But he is making program in Visual Basic. If you want respect, best code in Java.
    If you want respect code in Lisp or ML. If you want a reasonable job then you need to code in Java.
  • fred (unregistered) in reply to mrfr0g
    mrfr0g:
    Obligatory XKCD: http://xkcd.com/327/

    Are you using a plug-in? My browser doesn't support the XKCD: protocol

  • (cs)

    TRWTF is assigning the return value to a variable instead of returning from inside the loop immediately.

  • VetusDBA (unregistered) in reply to pjt33
    pjt33:
    The simple solution would appear to be to name all of your SPs things like insert_foo or update_bar.

    Amateur.

    SQL> create procedure "select" as 2 begin 3 null; 4 end; 5 /

    Procedure created.

    SQL>

  • Xyandir (unregistered) in reply to Anonymous Paranoiac
    Anonymous Paranoiac:
    This reminds me of an email validation routine we used to have at work that verified the address had both an "@" and a ".". It allowed such things as "foo@bar.", "foo.bar@", "foo@[email protected]", or just ".@". It also failed to strip even non-printable characters. I re-wrote the routine myself. Unfortunately, the platform was truly ancient and did not support regular expressions.

    To be fair, this seems like a very good way if you want to allow every valid adress without going insane. http://haacked.com/archive/2007/08/21/i-knew-how-to-validate-an-email-address-until-i.aspx But on the other hand, demanding a "." is also too much, as "admin@com" would be a valid adress for the TLD .com.

  • Uru (unregistered) in reply to Jay Enef

    This implementation would lead to some interesting workarounds. The main thing to notice is that each character is either even or odd, with the addition of even characters keeping the new string in the same class and odd characters causing a switch.

    Turns out spaces and newlines don't matter, because they're even in ASCII. Capitalization doesn't matter either, as there's an even number of characters between upper and lower case. However, tabs and semicolons are odd, so adding one to a failing statement will make it work and vice versa. Other good choices are adding stuff like comments, AS statements, or where clauses that are always true.

    Furthermore, you can pretty quickly tell if a particular statement will work by memorizing whether the most common words as a whole are even or odd. For instance, SELECT is even and thus has no impact on whether a statement works.

    There may be a strong bias towards either working or failing. I suspect failing will be more common (the required ";" at the end makes the simplest possible statement fail, and SELECT, FROM, and * are all even words).

  • Shinobu (unregistered)

    If this is anything like the codebases I've encountered, the function is only ever called with a variable as a parameter, whose value is supplied by code far, far away from the call site. And depending on the return value, different things get executed. Removing a function like this can take days.

  • Gibbon1 (unregistered) in reply to Smouch
    Smouch:
    Seeing as he recognized the total uselessness of the validator function, why did he add the execute keyword at all?

    Why not comment out the code and replace it with

    return true;

    Who knows what horrors lurk down in the dank depths of the code where no mortal has tread since the Clinton Administration. Perhaps this worthless function, as terrible as it is, is the only think keeping the world from falling to ruin.

    Oh yes remove it, and who knows what strange previously blocked SQL query will suddenly get executed against the database because this shabby knight no longer feebly guards the door.

  • (cs) in reply to warlaan
    warlaan:
    We store scripts that apply the necessary changes from one version to the next, but we don't store a complete creation script for the whole database schema.

    In theory that should be sufficient to tie the schema to the code version, but as our deployment tools do not automatically apply scripts to the database there are enough opportunities for human error.

    This is actually not the main reason why we don't use stored procedures - the main reason is that it was a company guideline not to use them. This is just the reason why we never felt much urge to break that rule.

    Someone help me please - I can't tell what the bigger WTF is - is it the company guideline not to used stored procedures, or is the developers who think putting tsql in their apps a better way to go?

  • (cs) in reply to lucidfox
    lucidfox:
    TRWTF is assigning the return value to a variable instead of returning from inside the loop immediately.

    That's actually a design pattern. A design anti-pattern, IMO.

  • ztrem (unregistered)

    Guys, this obviously isn't an anti-injection thing. If it were, they wouldn't allow "Delete". That's all you need to wipe the database of any actual data.

    If I were to speculate...

    Somewhere, some variable was getting set with a value that was either SQL or something else. And this function was created as a quick and dirty way to figure out whether it was SQL - the person who programmed this knew it wasn't perfect, but it was good enough for what they needed to do with it. And then someone else noticed the function and decided that everything that calls the database should use it, because validation is good. Nobody else even noticed, because they weren't using stored procedures at the time. And then, years later, someone tried to use a stored procedure, found they couldn't, shrugged, told everyone else that stored procedures wouldn't work on the codebase, and found another way to do things.

  • Dirk (unregistered)

    exec usp_select_your_mom

    Problems?

  • Rootbeer (unregistered) in reply to ZoomST
    ZoomST:
    The book was called this way long time ago, such as when Balrog was typing in Usenet.

    "Dear Balrog, how do you type with boxing gloves on your hands? Crapfully yours..."

  • (cs) in reply to Verisimilidude
    Verisimilidude:
    Nagesh:
    But he is making program in Visual Basic. If you want respect, best code in Java.
    If you want respect code in Lisp or ML. If you want a reasonable job then you need to code in Java.

    What is ML? Wiat, I will google that.

  • Sly Gryphon (unregistered) in reply to QJo
    QJo:
    FTFY:
    return int(rand(true/false))

    What you want to do is set up a map whose key is the command, and whose value is True or False. The first time

    But that requires storing a map. To avoid that, and still be consistent, calculate the MD5 hash and take the first bit to determine true/false.

    This would, however, be deterministic and allow the developer to determine in advance if their SQL statement would work or not... much like adding the comment ";--select" at the end would work with the original function.

    Captcha: consequat

  • Obligatory (unregistered) in reply to Anonymous Paranoiac
    Anonymous Paranoiac:
    This reminds me of an email validation routine we used to have at work that verified the address had both an "@" and a ".". It allowed such things as "foo@bar.", "foo.bar@", "foo@[email protected]", or just ".@". It also failed to strip even non-printable characters. I re-wrote the routine myself. Unfortunately, the platform was truly ancient and did not support regular expressions.

    It's a good job it didn't support regular expressions, otherwise you would've had two problems.

  • sqldev (unregistered) in reply to Tractor
    Tractor:
    Not unless someone actually writes an invalid query. The real WTF is validating queries yourself. The database does that just fine on its own, and even tells you what was wrong with it (most of the time).

    Not if you're using Oracle.

Leave a comment on “SELECTing Valid SQL”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article