$this->db->escape() - Fix.

[eluser]barryskidmore[/eluser]
All:

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

update table set quantity=quantity+1

Or

update table set updatetime=NOW()

So I modified the function slightly "DB_driver.php" line 687

Code:

/**
     * "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;
    }

[eluser]Colin Williams[/eluser]
Why would you be escaping these things anyway?

[eluser]barryskidmore[/eluser]
$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.

[eluser]Colin Williams[/eluser]
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...

[eluser]barryskidmore[/eluser]
I went ahead and restored that original and now only call it when required:

Code:

/**
     * 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-gui...lpers.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.

[eluser]Colin Williams[/eluser]
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).

[eluser]cmgmyr[/eluser]
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.

Code:

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);
    }