• Joseph Newton (unregistered) in reply to Jeff S
    Jeff S:
    Joseph Newton:
    Well Jeff,

    Your argument would be much more convincing if the sample code you used were error=free and tested. I just took a side-jaunt off to follow your link, and had a major WTF!?!? moment when I saw the "right way to do it" sample code blithely data intended for an ID column to a column named @TYPE.

    I you have time to write te lengthy screed, you could at least find some time to test your code against some actual test data. It's one thing to have typos when writing into these screwy undersized textareas on thedailywtf.com. It's just plain lazy to not check the integrity of your smples when holding forth as the expert on your own blog.

    /Captcha -- gotcha indeed, or you set yourself up for it.

    Wow. you got me. A cut and paste typo in my SAMPLE code which is simply an EXAMPLE and not designed or intended to run in any specific context (where's the using statements? the variable declarations? the call to the connection open method? the error handling? etc...) completely negates my entire point about a programming concept and methodology. Damn, I am an idiot. You got me.

    You know, it's people like you and posts like yours that really ruin what should be interesting comment/respond threads on forums like these and blogs like mine. It sure is fun when you try to take the time to intelligently explain/debate something and you get responses like "well, I must be right and you MUST be wrong because you typed 'its' instead of 'it's' on your post, so you are a moron! I win! " ... it's kind of sad...

    If you really don't get it, then that's too bad. If you do, then why not comment on the actual POINT of the post/article and not nitpick on completely meaningless things like easily fixed typos that are given in EXAMPLES? Or would that be too hard and take too much effort?

    Well, all I can say is that I disagree. Examples are what people learn from, and one well-constructed sample can generally communicate much more succinctly what a thousand words of rambling discourse only obscures.

    As to the gist of your article, well I agree in part--it is certainly better not to low-level functionality for which there are dependable existing functions available. I'm not at all sure that the framework you are trying to push qualifies.

    I recently did a short detour into the land of .NET, and it was truly fascinating. Being a good boy, I followed the tutorial examples character-for-character and attempted to run them. Almost all failed as written, for a variety of reasons. I was able to get all but two to work, but only by fixing them. Bear in mind that this were tutorials, intended to give the new user a picture of how the system worked.

    There is something inherently broken in this do-everything-for-you framework to which you evince such chauvinistic loyalty. I lost count of the times I ad to toggle the IIS framework specs for IIS folders back and forth between v2.x and 1.x.

    I much prefer in cases like this to take my chances on a bit of low-leveling, testing potential code directly into the mysql command-line interface with sample data, then substituting variable names for the params. Frequent compiliation and testing quickly reveals any bugs that have slipped through. I also use the available tools for escaping, of course. If the code fucks up, I at least know where to look for the problem, rather than guessing at where some damnfool MS engineer fucked up.

  • Leo (unregistered) in reply to AdT
    AdT:
    Leo:
    Did anybody consider that the user which executes the said statement against the DB only has the right to read?

    Then it might still be a confidentiality disaster.

    Because you could read a database that is meant to be read?

    Injection to get informations is possible, of course. But I fail to see the harm without more evidence than a gif. The addresses in this db are already on a page somewhere to be read by bots.

    AdT:
    Leo:
    Or what would be wrong with utilizing simply the daabase-security-measures to protect its content?"

    Pardon? In this case you would need a database user for every application user and per-row-ACLs, which few databases support (and which are horrible for performance, anyway).

    In this special case here you would need a database-user for the application which interfaces this database. So just add user "www-job-query" and set his rights to access the database according. Mayhaps on table-level if you are anxious to read-protect, a feature even the often shunned MySQL offers today.

    Sometimes it is good to be lazy and most DBS already offers the possibilities to protect against destructive injection.

  • Tom_fan_DK (unregistered) in reply to ObiWayneKenobi
    ObiWayneKenobi:
    This is not a WTF. I'm certain the code filters to avoid SQL Injections.

    Filters???? Still not getting it???? Can you all spell/shout BIND VARIABLES and use them properly? It's not so hard... :-(

  • Cheatz (unregistered) in reply to Sam

    Ah, a FRIEND'S website... sure, sure ;)

    captcha: muhahaha (? it's not THAT funny :P)

  • (cs) in reply to Joseph Newton
    Joseph Newton:
    Well, all I can say is that I disagree. Examples are what people learn from, and one well-constructed sample can generally communicate much more succinctly what a thousand words of rambling discourse only obscures.

    As to the gist of your article, well I agree in part--it is certainly better not to low-level functionality for which there are dependable existing functions available. I'm not at all sure that the framework you are trying to push qualifies.

    I recently did a short detour into the land of .NET, and it was truly fascinating. Being a good boy, I followed the tutorial examples character-for-character and attempted to run them. Almost all failed as written, for a variety of reasons. I was able to get all but two to work, but only by fixing them. Bear in mind that this were tutorials, intended to give the new user a picture of how the system worked.

    There is something inherently broken in this do-everything-for-you framework to which you evince such chauvinistic loyalty. I lost count of the times I ad to toggle the IIS framework specs for IIS folders back and forth between v2.x and 1.x.

    I much prefer in cases like this to take my chances on a bit of low-leveling, testing potential code directly into the mysql command-line interface with sample data, then substituting variable names for the params. Frequent compiliation and testing quickly reveals any bugs that have slipped through. I also use the available tools for escaping, of course. If the code fucks up, I at least know where to look for the problem, rather than guessing at where some damnfool MS engineer fucked up.

    huh .... ???

    That was quite a post. I don't even know how to respond. This makes a lot of sense ... you had trouble following some tutorials somewhere, so your conclusion: M$ suckz!!! Go MySQL !!

    Wow... Then again, it does make sense: you probably skimmed my entire blog post, missed all the points about parameters and about how not using them causes more issues rather than just the need to escape data, went right to some code that you hoped you could cut and paste and run, but it didn't, so all you learned from the article is that I must be an idiot and .NET must suck since the code didn't compile. Your experience with my blog post, now that I think about it, explains your comments pretty well I suppose ...

    I will suggest trying to LEARN the technology, READ and UNDERSTAND the articles and documentation, maybe even a book or two (reputable of course -- not some random guy's blog!) and don't just blindly think that all you need to do is skip all that boring "tech talk mumbo-jumbo" and just go right to cutting and pasting the code. There's a name for people like that: we call them script kiddies.

  • Diego (unregistered) in reply to Jeff S

    many sql frontends, like mysql_query() in php 5 don't allow two sql queries in the same string

    captcha: ewww ^^

  • ringbark (unregistered) in reply to mav

    Imitation is the sincerest form of flattery. Except where it's out-and-out plagiarism, of course.

    http://www.bash.org/?747235

    atari ? what sort of captcha is that?

  • (cs) in reply to CaptainObvious
    CaptainObvious:

    Never is such a horrible word to use. ...

    System.String.Format("SELECT * FROM Foo WHERE Bar={0}", SQLExcape(szUserInput));

    What does that get you that expr = prepare('...WHERE Bar=:1') followed by a good ol' bind(expr, szUserInput) && execute(expr) doesn't?

    Plus you can get and catch type mismatch exceptions at bind-time instead of execute time. So there's no excuse for varchar(100) columns everywhere you lazy assholes.

  • (cs) in reply to strcmp
    strcmp:
    Statements with variable tables have to be dynamic, at least the table names must be inserted as strings. Reusing the query plan only works if the tables stay the same, anyway. And if you use the query multiple times.
    If your application creates table names on the fly from user input then you need to seriously reconsider the architecture of your application and/or database schema. This indicates a PROBLEM... an impedence mismatch (if you will) between your object model and your data model.

    The query plan is always compiled once (no matter how the query is prepared or executed). Why not use constructs that promote potential reuse the second time without re-tooling? You don't lose anything.

    CREATE TABLE/CREATE INDEX/ALTER TABLE/ALTER SEQUENCE may need values as parameters, e.g. partition criteria or default values. Are they parametrizable in every database? Do they count as 'queries'?
    Another reason why dynamic table names are not a good idea; your application should not be executing DML during the course of non-maintenance or non-administrative operation. These details could be extracted into database-dependant maintenance scripts or routines.
    Variable length lists like "WHERE a IN (?,?,?,...,?)" where you have the choice between typing in or dynamically creating 'all' variants of the template. Or loading the values into a temporary table and JOINing.

    The join function available in nearly every language (coupled with varadic or array-accepting bind calls) make this a non-issue. lrn 2 use ur db api

    Other complicated logic may need to generate WHERE clauses totally dynamic depending on user input or config files (e.g. formulas or complicated criteria). It may be less error prone to directly insert the values and not construct a template string and parameter list independently.

    Constructing a query from a template is one thing. But never interpolate the value. That's just lazy. Most modern DB layers support "named" binds; that's what you should use instead (replace that '$' for a ':'). And consider building an associative array of the parameters (some of which may not actually be consumed by the resulting query); many DB API's support passing one in at bind time. You could do something fancy like mirror your configuration strings int the assoc array then ref them by name in the query.

    MAGIC!!!

    Some database protocols don't support parameters and the client side drivers have to fill in the parameters by string concatenation anyway. Depending on the circumstances it may be faster to do the concatenation yourself, e.g. if you call the statement multiple times with the same parameters.

    No database worth using has this limitation this decade. This includes MySQL and SQLite. Get real.

    Good client drivers have an encode() or quote() method that solves all locale and encoding problems because it does the Right Thing in any case.

    Did you make sure to properly set your LANG and database's language? How about in your web server, do you serve documents with local-sensitive javascript doing conversion on client-codepage encoded form boxes?

    Sending PREPARE, receiving the statement handle, sending parameters and receiving the result are 2 network round trips. Just sending a complete statement and receiving the result is 1. 50% speed up.

    Is your database located in a different state? What's worse than this by orders of magnitude is tearing up and shutting down database connections. I doubt you manage your connection pool very effectively if you can't figure out the rest of your database's API if you haven't gotten beyond rowset = execute(immediate) notation and feel the need to justify it.

  • bern (unregistered)

    Irony of ironies? Here while we're laughing at silly security gaps, there sites on the comments page of the previous Dice.com article, not too far down, a link to Ebaumsworld that downloads a Trojan. Sadly, I double as a Marketing guy, and I was curious about the hyperlink to "Marketingese" (virus downloading link omitted). A fraction too late, I repented of my internet sins and closed the browser, only to be greeted by Symantec's friendly virus-deleted screen.

    Yes, my mistake, but isn't it a bit silly to link to trojans on a very public website with a curious readership?

  • frank (unregistered) in reply to CaptainObvious
    Never is such a horrible word to use. I concat strings for SQL execution all the time

    But doing that (concatenation) would create a cursor for each distinct set of parameters, whereas using parameterized sql would enable cursors to be shared. A parse w/ cursor reuse is typically at least a magnitude faster than a parse where a new cursor is created (you have to check the syntax, do name/object resolution, check permissions on those objects, optimize the statement etc..).

    The bottom line is that using literal sql in applications is just downright stupid. It's widely agreed that this (use of literal sql) is one of the most common (and fatal) mistakes among sql developers.

  • bling (unregistered) in reply to drop database;
    drop database;:
    Unelegant! How about:
    bool breakOut = false;
    
    while (!breakOut) {
    try {
       this.RunTest();
    }
    
    catch(Exception e) {
       if(e.Message.IndexOf("SA Application") != -1) {
          continue;
    
       }  else {
          breakOut = true;
          throw e;
       }
    }
    
    }
    
    

    Do you consider you code elegant? Is elegance the only important matter? What about trying it to WORK? This will never end if suceeds.

  • Figs (unregistered) in reply to Wene Gerchinko

    I had a friend in 6th grade who defined cute as follows: "Ugly, but adorable."

    Is that what you meant? ;)

    captcha - 'sanitarium'

  • Darwin (unregistered) in reply to CaptainObvious
    CaptainObvious:
    Jeff S:
    You should *never* concatenate anything together with a SQL statement and execute it, unless the language/framework/database you are using doesn't support parameters. Of course, in that case, you probably have bigger issues to worry about.

    Never is such a horrible word to use. I concat strings for SQL execution all the time, well, I use System.String.Format, actually, but same effect:

    System.String.Format("SELECT * FROM Foo WHERE Bar={0}", SQLExcape(szUserInput));

    At least on Oracle (everyone's favorite database!), doing that forces a hard parse of the query every time, and will eventually push other parsed queries out of the cache, even if they used bind variables, thus forcing them to be hard parsed the next time they're executed.

    If you use parameters, Oracle can tell that it's the same query as last time (except for the particular values of the parameters), do a soft parse, and use the cached execution plan. You're killing your performance.

    File this technique under "How to make Oracle 10g perform like mysql 3."

  • Darwin (unregistered) in reply to kirchhoff
    kirchhoff:
    strcmp:
    Statements with variable tables have to be dynamic, at least the table names must be inserted as strings. Reusing the query plan only works if the tables stay the same, anyway. And if you use the query multiple times.

    If your application creates table names on the fly from user input then you need to seriously reconsider the architecture of your application and/or database schema.

    Definitely. I have rarely found a case where I needed to dynamically choose a table, but in the rare case (I have seen it), the solution is to have a few different parameterized queries, and choose the appropriate one, then apply the params as bind variables. (Don't ask, it was a bad design on the part of someone who came before me.)

    CREATE TABLE/CREATE INDEX/ALTER TABLE/ALTER SEQUENCE may need values as parameters, e.g. partition criteria or default values. Are they parametrizable in every database? Do they count as 'queries'?

    Another reason why dynamic table names are not a good idea; your application should not be executing DML during the course of non-maintenance or non-administrative operation.

    You mean DDL here, naturally.

    Variable length lists like "WHERE a IN (?,?,?,...,?)" where you have the choice between typing in or dynamically creating 'all' variants of the template. Or loading the values into a temporary table and JOINing.

    The join function available in nearly every language (coupled with varadic or array-accepting bind calls) make this a non-issue. lrn 2 use ur db api

    OK, this is one where I have had a problem, and I will admit that I broke down and consed up the query as a string. I first wrote code that would create the variable length parameterized query and a list of params, then wrote code that would loop over the params to add them. But I ran into a limit of 32 params in Oracle. I have to admit, I'm weak on temp tables, I guess I need to bone up on them.

Leave a comment on “Now Hiring SQL Injectors”

Log In or post as a guest

Replying to comment #:

« Return to Article