permitted_uri_chars in application/config.php not working [serious vulnerability]

[eluser]Keat Liang[/eluser]
Bitbucket Issue Tracker Link

some illegal character in URL is not block by the filter

= * , are blocked by the framework
but "^", "`", single quote, and double quote are not blocked by the uri/URL filter

tested with latest CodeIgniter Reactor build 6b1e35f45ca5

some how it broken and not working anymore....

[eluser]Twisted1919[/eluser]
How is this a security issue, could you share a line of code[exploit] to demonstrate the vulnerability ?

[eluser]Keat Liang[/eluser]
if i using pagination lib and my application is accept
and using active record,

controller

Code:

class Exploit extends SOME_dry_STUFF{

function __construct()
{
    //blah blah blah..... load necessary lib
    $this->load->model('news_letter_model');
}

function index($offset = 0)
{
    $data = array();
    //Configuration for pagination
    $limit = 10;

    //get all result
    $data['query'] = $this->news_letter_model->get_unique_visitor(
                         $this->current_site,
                         $search,
                         $limit,
                         $offset
                     );

    $total_num_result = $this->news_letter_model->count_unique_visitor($this->current_site, $search);

    $config = array(
        'base_url' => site_url('news_letter/visitor/index'),
        'total_rows' => $total_num_result,
        'per_page' => $limit,
        'uri_segment' => 4, //std in current structure
        'num_links' => 5
    );

    $this->load->library('pagination');
    $this->pagination->initialize($config);

    $data['page_links'] = $this->pagination->create_links();
    
    
    $this->load->view('visitor/index_view', $data);
}
}

model

Code:

class News_letter_model extends CI_Model{

    /**
     * Get List of Unique Visitor/ Count Total Unique Visitor [by search]
     * @param int $domain_id
     * @return mixed
     */
    function get_unique_visitor($domain_id = NULL, $search = "",
        $limit = NULL, $offset = NULL, $count = FALSE)
    {
        if($count === FALSE)
            $this->db->select('user_id, domain_id, users.username, users.name');
        else
            $this->db->select("COUNT(user_id) total");
        
        if($search != "")
        {
            $like_str = $this->db->escape_like_str($search);
            $this->db->where("(user_id  LIKE '%$like_str%' OR
                 username LIKE '%$like_str%' OR
                 name     LIKE '%$like_str%')");
        }
        
        $this->db->from('domain_unique_visitor')
                 ->join('users', 'domain_unique_visitor.user_id = users.id', 'INNER')
                 ->where(array(
                     'domain_id' => $domain_id
                 ))
                 ->limit($limit, $offset); // <!!!!!------ EXPLOIT this function does not excape or check for data type....

        $query = $this->db->get();
        
        if($query && $query->num_rows() > 0)
        {
            return $query;
        }
        
        return FALSE;
    }
    
    
    /**
     * Count Unique Visitor Wrapper Function
     *
     * @param int $domain_id
     * @param string $search
     * @return int
     */
    function count_unique_visitor($domain_id = NULL, $search = "")
    {
        $query = $this->get_unique_visitor($domain_id, $search, NULL, NULL, TRUE);
        
        return $query === FALSE ? 0 : $query->row()->total;
    }
    
}

request made
BTW the single quote is purposely put there to make SQL error

Code:

index.php/news_letter/visitor/ban/7'

error message will appear. i know i should turn off db debug, but sometime shit happen
IM using HMVC BTW

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 '', 10' at line 5

SELECT `owner_opt_out`.`user_id`, `users`.`username`, `users`.`name` FROM (`owner_opt_out`) INNER JOIN `users` ON `owner_opt_out`.`user_id` = `users`.`id` WHERE `domain_id` = 9 LIMIT 7', 10

Filename: /modules/news_letter/models/news_letter_model.php

Line Number: 234

some SQL injection tool try to do some evil thing,,,,
potential SQL injection

Code:

Analyzing http://localhost/index.php/news_letter/visitor/ban/1
Host IP: ~~~~~~
Web Server: Apache/2.0.63 (Unix) mod_ssl/2.0.63 OpenSSL/0.9.8e-fips-rhel5 mod_auth_passthrough/2.1 mod_bwlimited/1.4 FrontPage/5.0.2.2635 mod_perl/2.0.4 Perl/v5.8.8
Powered-by: PHP/5.3.3
Keyword Found: Visitor
I guess injection type is Integer?! If injection failed, retry with a manual keyword.
DB Server: MySQL
Findig columns count for MySQL failed!
Bypassing illegal union failed! Turning off this feature
MySQL time based injection method can't be used
Target Vulnerable :D

because of the url allow single quote which default does not.
but it allow it anyway.........

test under mac osx 10.6.7

[eluser]Keat Liang[/eluser]
btw another bug also filed

https://bitbucket.org/ellislab/codeignit...n-have-sql

[eluser]Keat Liang[/eluser]
bug demo, uri that does not block

http://www.websitedeveloper.com.my/ci/Co...ome/index/'"`^

uri that block

http://www.websitedeveloper.com.my/ci/Co...ome/index/!@$

[eluser]osci[/eluser]
I don't know if it should be escaped or not by limit.

But in your example you are protecting your search variable and not limit or offset or domain that you accept from the url. Shouldn't you escape everything you get from the url?

[eluser]Keat Liang[/eluser]
$this->db->limit($limit, $offset);

is belong to active record. and it should be automatically escape value.

i protecting the search variable because it is using custom where clause

Code:

$this->db->where("(user_id  LIKE '%$like_str%' OR
                 username LIKE '%$like_str%' OR
                 name     LIKE '%$like_str%')");

because of custom where clause is not escaped. manual escape is required.

these where clause below will automatic escape
since in the user guide says:
http://ellislab.com/codeigniter/user-gui...ecord.html
Beyond simplicity, a major benefit to using the Active Record features is that it allows you to create database independent applications, since the query syntax is generated by each database adapter. It also allows for safer queries, since the values are escaped automatically by the system

Code:

//auto escape yay !
$this->db->where('name', $name);
$this->db->where('name !=', $name);

$array = array('name' => $name, 'title' => $title, 'status' => $status);
$this->db->where($array);

//but not this
$this->db->limit(); // does not escape

by combine both uri exploit allow some illegal character and the limit function does not escape value
then other ppl assume is safe....

CI still awesome !

sorry for my poor english

[eluser]Keat Liang[/eluser]
[quote author="osci" date="1308261566"]I don't know if it should be escaped or not by limit.

But in your example you are protecting your search variable and not limit or offset or domain that you accept from the url. Shouldn't you escape everything you get from the url?[/quote]

i suggest the limit function(active record) should using is_numeric() to validate the data.

since it SQL LIMIT only accept INT

[eluser]osci[/eluser]
[quote author="Keat Liang" date="1308262408"]
i suggest the limit function(active record) should using is_numeric() to validate the data.

since it SQL LIMIT only accept INT[/quote]

True. Checked at mysql docs and LIMIT can be non negative integer with the exception of prepared statements and stored programs, which don't imply for active record.

[eluser]Twisted1919[/eluser]
You might be right, the limit() method does not seem to escape the values.
Even though the params given to this method should be integers, as you noticed, strings can be passed.
I do type casting for this method anyway[ie: (int)$limit, (int)$offset], but there might be developers who didn't do it, so it will be a security risk after all.
Hope somebody from the development team will look into this.