SQL Injection

[eluser]Unknown[/eluser]
I am new to CodeIgniter (CI) but I have inherited a CI project as the previous developer left our company. I am supposed to check and report whether the code is vulnerable for SQL injection (and if so rectify it).

For the user login page, the following code is used.

Code:

<?php
class login_model extends CI_Model{

        function __construct() {
        parent::__construct();
                $this->load->helper('url');
    }

         function login($username,$password)
                 {
                   $this->load->database();
                   $this -> db -> select('user_id, username, password');
                   $this -> db -> from('users');
                   $this -> db -> where('username = ' . "'" . $username . "'");
                   $this -> db -> where('password = ' . "'" . $password . "'");
                   $this -> db -> limit(1);

                   $query = $this -> db -> get();

                   if($query -> num_rows() == 1)
                   {
                         return $query->result();
                   }
                   else
                   {
                         return false;
                   }

                 }

If I give user name as 'David' and password as 'qwerty', I can successfully login to the web based application. That means, there is no error in the php or sql coding.

Therefore I tried the following as my password to attempted an SQL injection.

Code:

qwerty'; SELECT `user_id` FROM (`users`) WHERE `password` = 'qwerty

Then I got the following error, when I clicked the submit button:

Code:

A Database Error Occurred

Error Number: 1064

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'SELECT `user_id` FROM (`users`) WHERE `password` = 'qwerty' LIMIT 1' at line 4

SELECT `user_id`, `username`, `password` FROM (`users`) WHERE `username` = 'David' AND `password` = 'qwerty'; SELECT `user_id` FROM (`users`) WHERE `password` = 'qwerty' LIMIT 1

Filename: /var/www/html/uniclient/models/login_model.php

Line Number: 18

When I run the following on phpMyAdmin it gets executed successfully.

Code:

SELECT `user_id`, `username`, `password` FROM (`users`) WHERE `username` = 'David' AND `password` = 'qwerty'; SELECT `user_id` FROM (`users`) WHERE `password` = 'qwerty' LIMIT 1

That means there is no error in the way that I have crafted my SQL injection.

So why I get that error?

Is it because either our previous developer who left us and / or the CI framework is preventing SQL injection?

What I can see is that the code I inherited and now being tested is allowing SQL execution. It is definitely trying to execute SQL code input via the password field of the web form but for some reason execution fails.

Can we depend on this level of security (i.e. back-end tries to execute SQL statements input via the front-end by users but execution fails)?

Can't we prevent the back-end from executing SQL statements input by users via form fields?

If possible, how can we do it?

Any help is highly appreciated as I am helpless at the moment!!

[eluser]Aken[/eluser]
CI's DB library will only process one query per call. Running two queries creates the error.

Using Active Record, as is already there, will automatically escape content passed to it to prevent SQL injection. If you manually create queries, like you've tested, you need to perform the escaping yourself. You can use your own methods, or built-in ones such as $this->db->escape().

In your example code that uses $this->db->where(), it makes more sense to add the value as the second parameter, instead of creating a string like is there.

Code:

// No
$this -> db -> where('username = ' . "'" . $username . "'");
$this -> db -> where('password = ' . "'" . $password . "'");

// Yes
$this -> db -> where('username', $username);
$this -> db -> where('password', $password);

There's nothing technically wrong with the first way, but since there is no special comparison operator, it's just not really necessary.

More importantly, specifically for logging in, you should be performing some type of encryption of passwords, both for the sake of SQL injection, but more so for the protection if your users' vital information. Since your injected SQL is showing up plain as day, clearly that is not being done anywhere. Start here for more info: http://www.phptherightway.com/#password_hashing