Child pages
  • [security] Form mass assignment protection
Skip to end of metadata
Go to start of metadata
STRIDE

-

Damage potential0
Reproducibility5
Exploitability0
Affected users10
Discoverability0
DREAD Score3
5.1.xYes
RPIYes
Quote 

Every field, that is submitted from a form is considered as belonging to unit that processes it. This poses a security threat, because field absence on a form doesn't mean, that if added it won't be processed.

For example if attacker founds out what field gives user extra rights on website and just injects it to a form, then this would going to be saved directly to database without any validation at all. Luckily we already have kDBEventHandler::getRequestProtectedFields method, that allows to define which fields are in no-case allowed to be written from submitted form data into the database.

However I haven't seen anyone using that much, so the problem remains.

Maybe we need to use deny-by-default logic, where none of fields by default isn't allowed to be submitted from a form and developer must explicitly define which fields are allowed. Right now we using same policy with permissions: everything is denied and if developer decides to:

  • give user permission, that is bound to an event being executed
  • or by other means allows access to an event

then he will be responsible for that. Hopefully developer would think twice before allowing to set forged password reset token, that in theory would allow anyone to reset any other user's password using standard "Forgot Password" form.

Also I've found a video, which explains several form-related security issues and methods, used to solve them in Laravel framework:

Related Tasks

INP-1291 - Getting issue details... STATUS

3 Comments

  1. Also with getRequestProtectedFields we're kind of defining "black list" with a fields, that can't be set instead of defining "white list" which a fields, that can be set as done in Laravel. Downside of "black list" approach is that all added fields are automatically allowed to be set by mass assignment.

    Also we must keep in mind formatters, that for example create "DateField_date" and "DateField_time" fields for given "DateField" field. So if "DateField" is allowed to write, then also any fields, that are associated with it and created through formatter are too.

  2. Yes, I think it's pretty important topic, we should test and research suggested approach for form fields - denied-by-default.

    Alex, please create a task for this in 5.3.0 for now.

  3. Another problem with using white list of fields is that we need to explicitly add each field, that can be used on any form to white list. Without it all forms would stop working, because any values submitted from them won't be accessible in PHP side. This includes a virtual fields too.

    We can start by adding all fields and then narrowing down to the fields that are actually used or potentially are allowed to be used, but are not in use right now.