| # |
Aug 21st 2009, 16:51 |
gwoo |
feel free to hack your own code :) |
| # |
Aug 21st 2009, 16:50 |
gwoo |
from being able to use forms to do it |
| # |
Aug 21st 2009, 16:50 |
gwoo |
the goal is to prevent users |
| # |
Aug 21st 2009, 16:50 |
gwoo |
yes, developers will always be able to inject |
| # |
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? |