I've attached a patch to the ticket for review. Please comment. http://trac.symfony-project.org/ticket/8639
Thanks, Kris On 10 May, 12:08, Stephen Melrose <[email protected]> wrote: > It's not that simple Russ. > > Where do you perform the check that they are allowed to access/save an > object of a certain ID? > > I personally have always checked the object after I've taken it from the > route and before I've passed it to the form. After I've done that, I don't > expect the object to magically be transformed into another record, a.k.a. > something they're not allowed to access, and I bet the vast majority of > symfony developers don't either. > > Your point is valid and one I've agreed with throughout this thread, if > you're restricting what a user can edit, you need to make sure you safe > guard your code properly, and that's something I didn't do purely because I > didn't expect the scenario I've detailed to occur, and to be honest, nor > should it. > > The ID is passed as a hidden field (for whatever reason), but I don't expect > the PK to be changed. How often would you actually edit a PK? > > I'm simply arguing the PK should be read only be default. > > On 10 May 2010 20:01, Russ <[email protected]> wrote: > > > > > > > Personally I always check if the user has credentials to edit the > > object anyway and I couldn't give a monkeys if they change the id > > using Firebug or whatever as long as it's to one they have access to. > > If not, they'll get a nice 403 response either way. > > > The way I see it, editing the ID using Firebug or some other method > > would be just the same as if they opened that object up for editing in > > the first place... As long as they are allowed to, then so be it. > > > On May 10, 12:16 pm, Stephen Melrose <[email protected]> wrote: > > > 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? > > > > Stephen Melrose > > > > -- > > > 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]<symfony-devs%2bunsubscr...@google > > > groups.com> > > > 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]<symfony-devs%2bunsubscr...@google > > groups.com> > > For more options, visit this group at > >http://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 > 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
