This is why I'm so looking forward to Apostrophe's form builder plugin ;) On Tue, May 11, 2010 at 10:05 AM, Tom Boutell <[email protected]> wrote: > That does work Russ, but getting the id from the route - and > fancifying that by using a slug rather than an ID, and so on - is a > frequently encouraged Symfony practice. The problem is that the usual > examples of how to do this never acknowledge that the processForm() > method and its bind() call are going to get the ID from an entirely > separate source than the one we just saw explained in loving detail in > the Update or Create action. > > Since we want to encourage people to create gorgeous URLs (one of > Symfony's strongest features), we should also help them to do it > without creating unsafe sites. Having redundant ways the ID is being > passed is a recipe for security problems, and while I'm sure many > individual shops have worked around it (including ours), that's not > what frameworks are supposed to be about. (: > > In your own code, it sounds like you have a safe Update method. But > what about your Create method, where you're not expecting an ID at > all? Have you checked that a nonempty ID can't be passed there? > > On Mon, May 10, 2010 at 5:48 PM, Russ <[email protected]> wrote: >> Maybe I haven't read thouroughly enough, or maybe this is just to do >> with the methods involved here - but I normally look up the object >> using the id from $request->getParameter("my_form") or equivalent, >> then check the current user is allowed to modify it, then pass it to >> the form constructor. >> >> In this case, there is no way the "id" that I have used to retrieve >> the object, and the "id" that is in the bind() array can be any >> different, unless I do something weird in my own code. If someone >> changes the id in the hidden field (or form action url depending on >> the method) then that object will be looked up instead and credentials >> checked, again with the same ID that will eventually be passed to the >> bind array. >> >> Russ. >> >> >> On May 10, 9:08 pm, 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...@googlegroups.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...@googlegroups.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- Hide quoted text - >>> >>> - Show quoted text - >> >> -- >> 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 >
-- Blue Horn Ltd - System Development http://bluehorn.co.nz -- 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
