A good workaround in the meantime would simply be to document the issue, including in comments produced by the admin generator and CRUD generator.
On Tue, May 11, 2010 at 5:27 AM, Johannes <[email protected]> wrote: > A general fix needs some more thought. Looking at the possible fixes > that were suggested, these only work in limited scenarios when you use > relatively simple forms. > > For example, they don't work in the following cases: > - PK is not named id but something else, including composite PKs > - all embedded forms as bind is never called on them > > One could think simply unsetting id entirely would do the trick. > Unfortunately, some of the built-in validators rely on the id to > determine whether an object is new, or already exists, e.g. the unique > validators. What I do in these cases, is to simply not use the built- > in validator, but a callback validator which has access to the > underlying object directly. However, the unique validators could be > changed so that you can pass an object to the validator, and it would > not need the id for determining whether the object is new or not. > > Johannes > > > On 10 Mai, 21: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%[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]<symfony-devs%[email protected]> >> > 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 > -- 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
