Log message #990339

# At Username Text
# Aug 21st 2009, 16:50 kiger just to see what cake does
# Aug 21st 2009, 16:49 kiger UPDATE `products` AS `Product` SET `Product`.`total_products` = total_products + 1 -- WHERE `id` = 1
# Aug 21st 2009, 16:49 markstory why are you putting sql into integer fields?
# Aug 21st 2009, 16:49 kiger results in this:
# Aug 21st 2009, 16:49 kiger $this->Manufacturer->Product->save(array('Product'=>array('total_products'=>'total_products + 1 --')), false);
# Aug 21st 2009, 16:48 kiger $this->Manufacturer->Product->id = 1;
# Aug 21st 2009, 16:48 kiger mysqli
# Aug 21st 2009, 16:47 gwoo kiger: what dbo?
# Aug 21st 2009, 16:47 kiger lemme paste what I have (only a few lines):
# Aug 21st 2009, 16:47 kiger no I really have a "total_products" field; and that field allowed for the injection
# Aug 21st 2009, 16:47 markstory but they should be intval()
# Aug 21st 2009, 16:47 markstory well integers are not quoted...
# Aug 21st 2009, 16:46 proloser your username field is an integer field?
# Aug 21st 2009, 16:46 kiger my examples were using a string, but my real code was performed on a integer
# Aug 21st 2009, 16:46 kiger crud sorry; try it on an integer field
# Aug 21st 2009, 16:46 markstory doesn't do it for me.
# Aug 21st 2009, 16:45 gwoo kiger: around strings it would add 'test --'
# Aug 21st 2009, 16:45 gwoo updateAll does not
# Aug 21st 2009, 16:45 gwoo kiger: save adds the `
# Aug 21st 2009, 16:44 kiger So I tested out some injection on Model::save() and found this out ;(
# Aug 21st 2009, 16:43 kiger I was fooling with prepared statements using $this->query() last night (thanks for the patch btw markstory; it really kicks ass) but was curious about switching over to standard $this->save() and $this->updateAll(). I found it interesting that you cannot use prepared statements with updateAll() though which makes it very difficult to securely use; so I looked at what Model::save() does, but it just passes the stuff to update().
# Aug 21st 2009, 16:42 proloser hmm
# Aug 21st 2009, 16:40 kiger because the '--' comments everything else out afterwords (e.g., the WHERE id = 5)
# Aug 21st 2009, 16:40 kiger Here is the string I tried: $this->save(array('User'=>array('username'=>'test --')), false) which resulted in everyone's username being changed to 'test'
# Aug 21st 2009, 16:39 markstory updateAll is supposed to.
# Aug 21st 2009, 16:39 kiger well updateAll() clearly does, but doing an update through save() does too
# Aug 21st 2009, 16:39 kiger So I do $this->User->id = $userId
# Aug 21st 2009, 16:39 markstory so you're saying all update commands allow sql injection?
# Aug 21st 2009, 16:38 kiger I allow users to edit their account on my box
# Aug 21st 2009, 16:38 markstory ok
# Aug 21st 2009, 16:38 kiger [18:04] <kiger> $success = (bool)$db->update($this, $fields, $values);
# Aug 21st 2009, 16:38 kiger [18:04] <kiger> check line 1238 in the latest svn of model.php
# Aug 21st 2009, 16:37 markstory and should escape fields.
# Aug 21st 2009, 16:37 markstory Model::save() doesn't use updateAll
# Aug 21st 2009, 16:35 kiger But I never realized that Model::save() does not escape stuff either when you set Model->id because it will also use update.
# Aug 21st 2009, 16:34 kiger Basically, I didn't know that updateAll() doesn't escape stuff; the docs say it doesn't so no problem
# Aug 21st 2009, 16:32 kiger Should I mention it in here or does someone want me to explain in a pm?
# Aug 21st 2009, 16:32 kiger I think, and of course I'm probably wrong, I found a pretty big hole in cake regarding security that maybe many bakers don't know about?
# Aug 18th 2009, 10:24 kuja Ouch :|
# Aug 17th 2009, 12:53 ProLoser|Work1 !log
# Aug 16th 2009, 19:30 ProLoser !seen techno-dude