Hi Stephen,

Looking at what you're describing, I think this is "poor"
implementation (no offense meant). Form data is always vulnerable on
the clientside, so if you're using this data to update your record, be
aware that you have to check if the data is actually correct in one
way or another. If you want to restrict which records a specific user
can edit, then you'll have to implement some kind of protection for
that.

If you're using Propel, there is already a plugin that protects
against unauthorized row-access:
http://www.symfony-project.org/plugins/sfPropelRowLevelAccessBehaviorPlugin

I was discussing this with Pascal and he was mentioning we could
somehow protect hidden inputs, since they're hidden and not supposed
to change value: I see some strength in this, however this behaviour
should be optional since javascript can (and should) be allowed to
alter hidden values.

Another option Pascal mentioned is somehow protected primary key
hidden fields. I do see some value in this, though we have to look at
how best to implement this.

Interesting discussion for sure :)

Stefan

On Mon, May 10, 2010 at 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]
> For more options, visit this group at
> http://groups.google.com/group/symfony-devs?hl=en
>



-- 
Stefan Koopmanschap
Symfony Community Manager
[email protected]

-- 
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

Reply via email to