$this->db->escape() - Fix. | Libraries & Helpers

#1 / Sep 22, 2008 3:46pm

barryskidmore

7 posts

All:

$this->db->escape() has issues dealing with common databases functions such as

update table set quantity=quantity+1

update table set updatetime=NOW()

So I modified the function slightly “DB_driver.php” line 687

/**
     * "Smart" Escape String
     *
     * Escapes data based on type
     * Sets boolean and null types
     *
     * @access    public
     * @param    string
     * @return    integer        
     */    
    function escape($str)
    {    
        if (is_numeric($str) === false && stristr($str,'NOW()') === false) {
            $str = $str = "'".$this->escape_str($str)."'";
        }

        return $str;
    }

#2 / Sep 22, 2008 4:16pm

Colin Williams
2601 posts

Why would you be escaping these things anyway?
#3 / Sep 22, 2008 4:24pm

barryskidmore
7 posts

$this->db->insert_string() and $this->db->update_string() automatically call $this->db->escape() to quote.

Trying to use the helpers instead of just writing and escaping all my queries myself.

Considering I prefer to use database functionality over PHP where possible; I will probably stop using the DB helpers as they remove a layer of complexity in exchange for a layer of overhead.

I can however seeing someone making the same mistake I made and wondering why.
#4 / Sep 22, 2008 4:34pm

Colin Williams
2601 posts

I think it would be misuse of the helpers to pass operators and SQL functions to methods that escape all the input. But, yeah, it absolutely doesn’t surprise me that the mistake is made…

#5 / Sep 22, 2008 5:52pm

barryskidmore

7 posts

I went ahead and restored that original and now only call it when required:

/**
     * Generate an insert string
     *
     * @access    public
     * @param    string    the table upon which the query will be performed
     * @param    array    an associative array data of key/values
     * @return    string        
     */    
    function insert_string($table, $data)
    {
        $fields = array();    
        $values = array();
        
        foreach($data as $key => $val)
        {
            $fields[] = $key;
            if (is_numeric($key) === false && stristr($val,'NOW()') === false && stristr($val,$key) === false) {
                $values[] = $this->escape($val);
            } else {
                $values[] = $val;
            }
        }    
        
        return $this->_insert($this->prep_tablename($table), $fields, $values);
    }

Fits my needs and saves me abit of typing. Could easily be extended to check an array for all SQL keywords but I do not need to be that specific.

Any exploration of automating the preparation of data so as to severly limit injection attacks is always worthwhile, especially when the documentation offers little in the way of technique explanation.

http://ellislab.com/codeigniter/user-guide/database/helpers.html

This function simplifies the process of writing database inserts. It returns a correctly formatted SQL insert string. Example:

Note: Values are automatically escaped, producing safer queries.

#6 / Sep 22, 2008 6:07pm

Colin Williams
2601 posts

Actually, perhaps it does make sense in this context. I was coming from the thought that you should only escape user supplied data. However, using a helper to “simplify the process” doesn’t necessarily imply you are only working with user-supplied data. Hence, your second approach makes a lot more sense (however, it’s not terribly robust, since it only ignores one of many SQL functions).

#7 / Nov 14, 2008 12:04am

cmgmyr

47 posts

Glad I came across this post, I needed this. Thanks! I’ve updated the update_string function for this too in case anyone needed it.

function update_string($table, $data, $where)
    {
        if ($where == '')
        {
            return false;
        }
                    
        $fields = array();
        foreach($data as $key => $val)
        {
            if (is_numeric($key) === false && stristr($val,'NOW()') === false && stristr($val,$key) === false) {
                $fields[$key] = $this->escape($val);
            } else {
                $fields[$key] = $val;
            }
        }

        if ( ! is_array($where))
        {
            $dest = array($where);
        }
        else
        {
            $dest = array();
            foreach ($where as $key => $val)
            {
                $prefix = (count($dest) == 0) ? '' : ' AND ';
    
                if ($val !== '')
                {
                    if ( ! $this->_has_operator($key))
                    {
                        $key .= ' =';
                    }
                
                    $val = ' '.$this->escape($val);
                }
                            
                $dest[] = $prefix.$key.$val;
            }
        }        

        return $this->_update($this->prep_tablename($table), $fields, $dest);
    }

Thread