top | item 4138663

(no title)

x1 | 13 years ago

> 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

discuss

order

zopa|13 years ago

> "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...

jaylevitt|13 years ago

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?

papsosouid|13 years ago

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.

jaylevitt|13 years ago

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.

Peaker|13 years ago

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).