Comment by zopa | Hacker News Reader

Comment by zopa | Hacker News Reader

Parent Full thread

> "The only thing the compiler will yell at you is if you passed a type that is not of a Query type. The compiler will not yell at you for getting the current session directly or creating your own jdbc driver for that matter."

In Haskell, I'd have a module, Database, that held all my db code. That module would export functions something like

query :: Query -> DBResult update :: Query -> DBAction -> DBResult

(read those as "query is a function that takes a Query and returns a DBResult.")

In the rest of my program, those functions would be the only way to talk to the database. There's your guarantee.

Could I, rather than using my nice database module, instead drop into IO and write code to do something vicious? Surely. But now we've moved beyond bugs and into active malice.

> "Again, so this cannot be done in a dynamic language? If it can be done, why bring them up?"

It's harder. With duck typing, if it looks like a Query it is a Query, no? Even if it drops your table. I'm no expert on dynamic languages, and I'd believe that there are sophisticated object hierarchies that can do these things (at runtime...), but the original article is empirical evidence that real projects get this wrong.

Really, though, try a language with a modern type system and see for yourself. I know we Haskell users sound like zealots, but the difference between the Java and Haskell type systems truly is night and day.

> In the rest of my program, those functions would be the only way to talk to the database. There's your guarantee. Honest question. Take these pseudo sql calls:

    //Bad Person
    username = "lastname'; drop table user--"
    
    //Good Programmer
    query = "select * from users where name like %[username]%";
    input = {"username":"frank"};
    result = execute(query,input);
    
    //Bad Programmer
    query = "select * from users where name like '%"+username+"%'";
    result = execute(query, {});

vs

    //Bad Person
    String username = "lastname'; drop table user--"

    //Good Programmer
    Query q = new Query("select * from users where name like %[username]%");
    Input input = new Input(username);
    q.addInput(input);
    Result r = q.execute();
    
    //Bad Programmer
    Query q = new Query("select * from users where name like '%"+username+"%'");
    Result r = q.execute();

Could you solve this better using a static system? Right now I see no difference between the good and bad

In your static example, "Bad Programmer" would be fine, because the Query constructor does escaping. You could do this in a dynamically typed language too, but notice that you don't, you just use strings. The difference between static and dynamic is that with static typing, you can't compile your incorrect program. With dynamic typing, you find out at run time that you forgot to escape the string (turning it into a Query), when that code actually runs.

I'm admittedly ignorant of any type system newer than C++. In a modern static language, how would you design Query such that any SQL injection is caught at compile-time?

On the dynamic side, Rails (in Ruby) doesn't currently catch SQL injections, but it does catch HTML-escaping injections. It (roughly) tags all strings as tainted by default, and when you send them to the browser, it escapes them. If you want to send literal ampersands, angle brackets, etc., you have to mark them as explicitly safe. Since most of your literal HTML is generated by templates (which themselves distinguish variables from static HTML), you end up with run-time safety unless you actively try to break out of it.

See http://news.ycombinator.com/item?id=4139798

If he builds the final query string before giving it to Query, his valid query parts that rely on not being escaped would also be escaped.

To make a safe query type you'd have to provide non-string primitives to build one, if I understand correctly. You can't allow just a full query string (with all of the injections already in place) to be converted to a Query type (as in his Bad Programmer example).

> "Right now I see no difference between the good and bad"

You're building a new query string each time you create a Query object, and concatenating the string onto that. With that approach, each time you build a Query object you have a fresh opportunity to mess up. So you're right that there's no difference between your to cases.

Let's drop my off-the-cuff example and look at how a real library, postgresql-simple, handles the issue:

  query :: (ToRow q, FromRow r) => Connection -> Query -> q -> IO [r]

Usage example

  query conn "select x from users where name like ?" (Only username)

Do you see the difference? Instead of sticking the username into the SQL query by hand, we use a query function that takes three parameters: a database handle, a Query with a '?' character, and a thing you want to use in the query. The function takes care of properly escaping the username during interpolation. (The "Only" is just a wrapper to make sure we're handing in a datatype we can query with.)

Notice that because Query is a distinct type from String, just doing

  query conn ("select x from userse where name like" ++ username)

doesn't typecheck. Bad Programmer would have a hard time screwing this up.

The full documentation for postgresql-simple is here: http://hackage.haskell.org/packages/archive/postgresql-simpl...

Sure, and Rails offers a similar syntax:

  User.select('x').where('name like ?', username)

But if your language allows literal string interpolation (as Ruby does), what prevents you from doing this:

query conn ("select x from users where name like #{username}")

How do type-safe languages prevent this?

Query isn't a String. String interpolation[^1] would de-sugar to something like this:

  query conn ("select x from users where name like " ++ username)

++ is a function that expects two Strings. The "select..." stuff isn't a String, quotation marks not withstanding. When we try to hand a Query to ++, the compiler screams bloody murder.

Longer explanation: I suspect the syntax is a bit confusing, since while I keep saying "select ..." is a Query, it looks an awful lot like a String. Here's what's going on. Haskell has a typeclass called IsString. Query is an instance of IsString, as is String.[^2]

Quoted text can represent any instance of IsString. So the compiler sees a function that expects a Query and an IsString of some sort, and through the magic of type inference, it decides that the IsString must be a Query.[^3] And when you try to use a function that concatenates Strings on that Query, it knows that Something's Not Right.

[1]: Haskell doesn't have string interpolation. But if it did, this is how it would work.

[2]: And other instances as well. postgresql-simple actually uses ByteStrings, not Strings, for performance.

[3]: I've fuzzed the evaluation order a bit, for simplicity. In practice the first error reported might be that you've passed 2 arguments to a function that expects 3.

Reply on news.ycombinator.com