Stephen, can you elaborate a little? Did you add that validator yourself for some reason? If it's a validator on the id field and you unset the id field from the form, it shouldn't fire.
On Mon, May 10, 2010 at 10:05 AM, Stephen Melrose <[email protected]> wrote: > I tried removing the ID field, but sfValidatorPropelUnique stopped > working correctly. > > Instead, we put some code in the BaseFormPropel that changed the > binded ID back to the ID of the object, effectively rendering it read > only. > > On 10 May, 13:12, Tom Boutell <[email protected]> wrote: >> This is a dangerous behavior of the standard Symfony Doctrine and >> Propel CRUD module generators (and the admin generator as well). >> >> The Doctrine admin generator and CRUD generator (presumably Propel >> too) generate redundant code in which the object is fetched based on >> an ID in the route, but the form still contains an ID. >> >> Sure, the generated code gets away with this because it has no per-row >> validation anyway, but it still does not make sense and it is >> dangerous because the conspicuous IDs in the generated CRUD code mask >> the presence of the "sneaky," redundant ID field in the form. The >> minute you try to add any validation of who's supposed to be doing >> what at the controller level, you've got a big security hole and you >> don't know it. >> >> The CRUD generator and admin generator should generate form subclasses >> that unset the ID field, unset it directly at the action level, or >> verify that it's the same as the ID coming from the route and ignore >> it entirely in the create action, IMHO. Otherwise it's misleading and >> a danger in exactly the way you suggest. >> >> In our own projects we unset($this['id']) in the configure method of >> our Doctrine forms. >> >> 2010/5/10 Michał Piotrowski <[email protected]>: >> >> >> >> > Hi, >> >> > 2010/5/10 Stephen Melrose <[email protected]>: >> >> Hi, >> >> >> We have discovered what could be a potential flaw in the form >> >> framework. The reason I'm discussing this here is because I'm in mixed >> >> feelings as to whether this is actually bug or not, or rather poor >> >> implementation on our part. Either way, I'm also saying this flaw >> >> should be safe guarded against. >> >> >> We discovered that a malicious user can use the forms generated by the >> >> form framework to edit content they shouldn't be able to. >> >> >> They do this by replacing the primary ID in the hidden form field with >> >> that of the record they want to edit. When they hit save, the >> >> validation is run, and the Object is updated with the new ID, so when >> >> the save() is called, the other row is updated. >> >> >> Now, if we (as in developers) want to restrict editing of content for >> >> certain users, then it is our responsibility to make sure we put safe >> >> guards in place. I'm not arguing this fact. >> >> >> The reason I believe this to be a problem is how users will actually >> >> guard their code. Most people (including myself) run all the safe >> >> guard checks before the Object is passed into the Form on >> >> construction. I don't then expect the POST data to override the >> >> primary key of the Object on save. Infact, I can't think of an >> >> instance I would ever want this to happen. >> >> >> I therefore propose that some sort of restriction/block is put in >> >> place by default that stops the PK of an Object being altered on >> >> bind(). >> >> >> Thoughts? >> >> > I create a methods like newCommon() or editCommon() with all safe >> > checks and call them from new/create, edit/update. >> >> > The main reason for this is that you _always_ _need_ to perform the >> > same checks in new and create as well in edit and update. Why? >> >> > For example - user want to create a comment to blog post >> > - new method is called - all safe checks pass well >> > - form is rendered >> > - user write his comment >> > - other user delete his blog post >> > - user tries to write his comment >> >> > If you wont do the same check in create method you failed :) >> >> > IMHO it's a security vulnerability, but it's not symfony fault. >> >> > And BTW. CSRF protection should do the trick for form protection >> >> >> Stephen Melrose >> >> > Regards, >> > Michal >> >> > -- >> > If you want to report a vulnerability issue on symfony, please send it to >> > security at symfony-project.com >> >> > You received this message because you are subscribed to the Google >> > Groups "symfony developers" group. >> > To post to this group, send email to [email protected] >> > To unsubscribe from this group, send email to >> > [email protected] >> > For more options, visit this group at >> >http://groups.google.com/group/symfony-devs?hl=en >> >> -- >> Tom Boutell >> P'unk Avenue >> 215 755 1330 >> punkave.com >> window.punkave.com >> >> -- >> If you want to report a vulnerability issue on symfony, please send it to >> security at symfony-project.com >> >> You received this message because you are subscribed to the Google >> Groups "symfony developers" group. >> To post to this group, send email to [email protected] >> To unsubscribe from this group, send email to >> [email protected] >> For more options, visit this group >> athttp://groups.google.com/group/symfony-devs?hl=en > > -- > If you want to report a vulnerability issue on symfony, please send it to > security at symfony-project.com > > You received this message because you are subscribed to the Google > Groups "symfony developers" group. > To post to this group, send email to [email protected] > To unsubscribe from this group, send email to > [email protected] > For more options, visit this group at > http://groups.google.com/group/symfony-devs?hl=en > -- Tom Boutell P'unk Avenue 215 755 1330 punkave.com window.punkave.com -- If you want to report a vulnerability issue on symfony, please send it to security at symfony-project.com You received this message because you are subscribed to the Google Groups "symfony developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/symfony-devs?hl=en
