Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

database functions get escaped as if they were strings #1895

Closed
entomb opened this Issue · 16 comments

4 participants

@entomb

When inserting data into the database, mysql functions get escaped as strings.
during insert or update when i want to use "NOW()" as a value.

example:

$this->db->insert('table',array(
                'id' => 1,
                'name' => "this is a name",
                'date' => "NOW()",
            )
        );

when using this statement the "NOW()" get escaped and the date ends up being null.
this is also true for any other statement.
Another example is the use of DATE_ADD or DATE_SUB on WHERE statements.

$this->db->delete('table',array(
                'date' => "DATE_SUB(now(), interval 1 day)"
            )
        );

this should work, but instead my statement gets escaped.
I suggest a way of forcing a non-escaped value or a white-list dictionary of allowed functions to skip the regular string escape functions

@GDmac

Which version of CI are you using?
Some DB methods accept an extra parameter to not try and protect identifiers.

This works for me for 'where':

// see user_guide third param on where
$this->db->select('session_id');
$this->db->where('`last_activity` < ', 'UNIX_TIMESTAMP(NOW())', FALSE);
$query = $this->db->get('ci_sessions');
var_dump($query->result());

db->insert() doesn't allow escaping directly (see manual). For INSERT use the db->set() method.

// false = not escaped
$this->db->set('last_activity', "UNIX_TIMESTAMP(NOW())", FALSE);
// others in associative array
$this->db->set(
  array(
    'session_id' => 10,
    'user_agent' => "blah",
  )
);
$result = $this->db->insert('ci_sessions');
var_dump($result);

"if all else fails, read the manual"

@entomb

What happens if I don't want to use active record, or if i'm using insert_batch()?

@GDmac

I don't know what happens if you don't want to use active record.

Have you looked at or tried regular queries?
http://codeigniter.com/user_guide/database/queries.html

insert_batch again 'is' an active record method. a quick google turned up
"set $this->db->_protect_identifiers to FALSE before your query? add any backticks yourself?"

@entomb

The problem is that i want to turn of the protection only in one of the array values, the rest should be treated normaly.

Ive added a simple solution for myself on the CI_DB_driver->escape() method so it works for insert() update() and delete() as well as the where parameter on those methods.

so I can do this:

$this->db->insert('table',array(
                'id' => 1,
                'name' => "this is a name",
                'date' => array("NOW()"),
            )
        );

and know the NOW() function will be passed correctly as a function and not as a string "NOW()".
its not an elegant solution, thats why I submitted this issue, in case someone else has a better work-around.

@GDmac

As a work-around you could also, for the batch, calculate the NOW() and DATE_SUB() with php.
or even get the mysql values beforehand with a simple query if you have elaborate date functions.

SELECT 
  NOW() AS mysql_now, 
  DATE_SUB(now(), interval 1 day) AS mysql_yesterday
@entomb

Stressing the database with queries is not a very good solution, as well as the possible date missmatch on slow batches.
With the escape() "hack" i'm fixing all problems in all methods, so i'm pretty happy with it for now,

Thanks for the feedback, It has been helpfull

@GDmac

On slow +1 second queries the NOW() might that not actually jump a second during insert / update, no?
Also, if i recall, form input can also be made an array fairly easy, by just adding two square brackets[] to the name of an input, you don't want those unescaped.

@entomb

Yes, this is a security issue for anyone who doesn't verify and sanitize form data properly.

Got any Ideas on how to do this without a huge security hole?

@GDmac

maybe someone more knowledgable than me (@narfbg , @dchill42 , @pkriete ) can look into this. Wanting to use native MySQL functions is not uncommon.

@narfbg
Owner

Added an optional escape parameter to insert() and insert_batch(), as you can see in the above commit. Additionally, this was still possible with the use of set() (which already had optional escaping):

$this->db->set('id', 1);
$this->db->set('name', "this is a name");
$this->db->set('date', 'NOW()', FALSE); // note the third argument
$this->db->insert('table');
@narfbg narfbg closed this
@GDmac

That makes it an all or nothing option then.
remember to escape() any fields from outside sources

@entomb

Thanks for this update, it behaves perfectly now.

@dev-david

I am having problems using now() for an insert_batch.

$insert_data = array();

    foreach($form_post_data as $guest_id)
    { 
        $insert_data[]= array(
        'owner' => $this->session->userdata['id'],
        'guest_id' => $guest_id,
        'event_name' => $this->session->userdata['event_name'],
        );
    }

    $this->db->set('created_at', 'NOW()', FALSE);
    $insert_user_query = $this->db->insert_batch('my_table', $insert_data);

I have used the NOW(), FALSE method before and it works fine for single inserts but I am new to code igniter and the insert_batch option. Any ideas?

@entomb

Hello David,

I solved this "tinny" issue of mine with the help of PHP, this might help you too:

 $insert_data[]= array(
        'owner' => $this->session->userdata['id'],
        'guest_id' => $guest_id,
        'event_name' => $this->session->userdata['event_name'],
        'created_at' => Date("Y-m-d h:i:s"),
    );

Using Date("Y-m-d h:i:s") will set a date that mysql can read as a datetime type of string

All mysql date function can more or less be emulated this way with the PHP date functions.

note depending on your configs, the string might need to be different, this is the default

@dev-david

Yes this was one of the first things I considered doing but really wanted to see if there was a CI way of avoiding this without going into system files etc etc.

Ended up using the Date() method in the end like you did but trying to see if there is a better way.

@entomb

there's a third parameter, as referenced in this issue that turns on and off the auto escaping. I wouldn't recommend if you have user inserted data going trough your query unless its a controlled query and the user is you (eg: a cron job, batch script, etc).

The Date() method is what I ended up using and I'm still happy with it.

@nonchip nonchip referenced this issue from a commit in nonchip/CodeIgniter
@narfbg narfbg Add an optional escape parameter to insert() and insert_batch()
"Fixes" #1895
d1e13c7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.