| # |
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 |
| # |
Aug 16th 2009, 18:11 |
proloser |
!seen ad7six |
| # |
Aug 15th 2009, 15:56 |
sky_l3ppard |
hi poLK, i fixed the article, please take a look, when you have time, thanks |
| # |
Aug 15th 2009, 06:36 |
ADmad |
i kinda got lost what was your take on this ? |
| # |
Aug 15th 2009, 06:36 |
alkemann |
well at least he has more content than the other one, but i must stick to our discussion of yesterday |
| # |
Aug 15th 2009, 06:34 |
ADmad |
ya |
| # |
Aug 15th 2009, 06:34 |
alkemann |
oh by Jon |
| # |
Aug 15th 2009, 06:34 |
ADmad |
yup suddenly people are too interested in that topic :) |
| # |
Aug 15th 2009, 06:33 |
alkemann |
another one? |