Monday, July 30, 2012

Defend against SQL Injection using ActiveJDBC

I was asked on forums how ActiveJDBC defends against SQL Injection attacks. My first reaction was: hey, this is not an ORM problem, talk to web devs :)
Luckily, Lukas Eder wrote a nice blog that helped put things into perspective. While SQL Injection attacks are not the task of ORMs per se, ORMs still need to provide some level of advice on how to deal SQL injection pitfalls.
Lukas even offered some actual code snippets on his article. What is a developer to do to try things out?  HE/she writes tests, and so I proceeded to write a test using some of the examples of his code.
The first test had this code:

String id = "1';drop table users;select * from people where name = 'John";
List<User> users = User.where("id = '" + id + "'");
User user1 = new User();
user1.set("first_name", "John", "last_name", "Doe", 
           "email", "").saveIt();
At first when I tried this against MySQL, I got "invalid statement" exception, apparently it could not handle the semicolon in the middle of the statement. After that, I ran this same code against H2 database, and sure enough, the SQL injection worked. This resulted in an exception:

Caused by: org.h2.jdbc.JdbcSQLException: Table "USERS" not found; SQL statement:
INSERT INTO users (email, first_name, last_name) VALUES (?, ?, ?) [42102-171]

Holy crap! My table USERS is gone!

This is bad news for those people who use a simple string concatenation to make SQL statements in public facing web projects.

However, I quickly wrote a second test that uses dynamic parameters passed to a model:

String id = "1';drop table users;select * from people where name = 'John";
List<User> users = User.where("id = ?", id);
User user1 = new User();
user1.set("first_name", "John", "last_name", "Doe", 
        "email", "").saveIt();
As you can see here, the only difference is that I pass the "name" parameter as a dynamic one, instead of simply concatenating strings

The output from this test is:

Model: org.javalite.activejdbc.test_models.User, table: 'users', attributes: {, FIRST_NAME=John, ID=1, LAST_NAME=Doe}

As you can see, the table USERS is intact, no harm is done.

So, what is the take out from here? String concatenation to build dynamic queries in web applications is  evil!

But, since ActiveJDBC uses PreparedStatement, you are safe as long as you use dynamic parameters instead of splicing strings together. 

I actually never use string concatenation, not because I constantly worry about SQL Injection attacks, but simply because this makes for some ugly spaghetti code and this probably one of the reasons I did not pay much attention to it before.

Cheers to safe coding!


Basil 3ibs said...

Hello Igor,

First, thanks for the wonderful ActiveJDBC library.

I'm having this problem in that Jenkins does not include the instrumented model classes in the final war file. I'm using a maven project that is compiled using a Jenkins job.

I have posted a question on regarding the ActiveJDBC with Jenkins problem:

Would be great if you could provide me with a hint on this problem.

Basil Musa

Igor Polevoy said...

Basil, I replied to your question on Stackexchange:

Basically, you need to see if instrumented classes are recompiled again for some reason