2

I would like to find an optimal way how to mitigate SQL injection in the web application developed in CodeIgniter framework (web application uses MS SQL backend DB with ODBC connection).

Let's assume I have a simple vulnerable code like this:

$this->db->query("SELECT * FROM users WHERE Login = '".$_GET['name']."'");

This query is clearly vulnerable to SQL injection through HTTP GET parameter "name".

I have read CodeIgniter documentation and everything I could find online to see how to mitigate this simple SQL injection in CodeIgniter and I have tried all of the following options:

Option 1:

$this->db->query("SELECT * FROM users WHERE Login = ".$this->db->escape($_GET['name']));

Option 2:

$this->db->select("*")->from("users")->where('Login', $_GET['name'])->get();

Option 3:

$query = "SELECT * FROM users WHERE Login = '?'"; 
$this->db->query($query, array($_GET['name']));

Option 4:

$query = "SELECT * FROM users WHERE Login = ?"; 
$this->db->query($query, array($_GET['name']));

I was shocked to find out that all four aforementioned options are just as vulnerable to SQL injection as the initial query. I was wondering whether CodeIgniter is so poorly designed from the security perspective or if I am missing some important piece of configuration.

Is there any conceptual way how to prevent SQL injection in CodeIgniter in this case?

Martin Pozděna
  • 41
  • 3
  • How exactly did you find that all options are vulnerable? The first two don't even work, the 3rd is just a wrong way of doing the 4th ... and the 4th is both the only "option" that works and the only one that is secure. – Narf Jun 09 '17 at 11:39
  • whats wrong with Option 2 ? – Atural Jun 09 '17 at 11:45
  • I have done a penetration test of application which has the vulnerable line of code mentioned in my post. Now, I would like to propose solution how to fix it. I tested all four options from the post, they all worked to me - but that is actually not important. If I use option 4 that is supposed to be secure and I submit the following HTTP GET parameter ?name=nonexistent' OR 1=1 then application throws the following error: [Microsoft][ODBC SQL Server Driver][SQL Server]Unclosed quotation mark after the character string ''. SELECT * FROM auth_user WHERE Login = 'nonexistent' OR 1=1' – Martin Pozděna Jun 09 '17 at 11:53
  • Are you sure, that 4th option isn't secure? 3rd option actually shouldn't work. You should write `?` instead of `'?'`. The reason for this is because integer values don't need to be escaped, but string values will be automatically escaped by CodeIgniter and then surrounded by your quotes. Also, this question is possible duplicate of https://stackoverflow.com/questions/5857386/how-to-avoid-sql-injection-in-codeigniter – berserkk Jun 09 '17 at 12:19
  • I opened this question because nothing in https://stackoverflow.com/questions/5857386/how-to-avoid-sql-injection-in-codeigniter works for me. I replaced original vulnerable code with option 4 and I am able to do exactly same sql injections as before. Maybe it has something to do with backend MS SQL and ODBC. Not really sure... – Martin Pozděna Jun 09 '17 at 12:26
  • OK I identified the issue now. It seems that this website uses legacy version of CodeIgniter framework in version 2. Proper escaping for MS SQL has been added in version 3. – Martin Pozděna Jun 09 '17 at 12:52

2 Answers2

2

OK after several hours of fighting this issue I managed to find an answer myself.

There was SQL injection vulnerability in CodeIgniter ODBC driver prior to version 3.1.0. So even if the coding style (Query Bindings) is correct from security standpoint (as demonstrated in Option 4) the application is still vulnerable to SQL injection through user supplied input.

I verified that by installing both CodeIgniter 3.0.6 and 3.1.0 side by side running the same query. Code in CodeIgniter 3.0.6 was still vulnerable to SQL injection while the one running in CodeIgniter 3.1.0 was not.

CodeIgniter change log https://www.codeigniter.com/userguide3/changelog.html

So lesson learned for the future. Always remember to check framework version as well.

Martin Pozděna
  • 41
  • 3
  • ... and update it regularly. Don't leave it at 3.1.0 - it's the one that fixes this particular issue, but not the latest. – Narf Jun 09 '17 at 20:10
1
  • check every single user input for datatype and where applicabile with regular expressions (never trust user input)
  • use prepared statements

it's a lot of work especially if your application is in bad shape but it's the best way to have a decent security level

the other way (which i'm advising against) could be virtual patching using mod_security or a WAF to filter out injection attempts but first and foremost: try to write robust applications (virtual patching might seem to be a lazy way to fix things but takes actually a lot of work and testing too and should really only be used on top of an already strong application code)

Alex Mac
  • 2,970
  • 1
  • 22
  • 39