From 6348f2e3e849da572f608b5dd12cccbc4e797eb4 Mon Sep 17 00:00:00 2001 From: Antony Thorpe Date: Tue, 6 Jun 2017 18:14:59 +1200 Subject: [PATCH] Updated Form.php & 04_Form_Security.md Changed the `strictFormMethodCheck` protected property from false to true to step out on the front foot with this security setting. In the documentation under the title [Cross-Site Request Forgery](https://github.com/silverstripe/silverstripe-framework/blob/master/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md#cross-site-request-forgery-csrf) it states, "it is also recommended to limit form submissions to the intended HTTP verb (mostly GET or POST) through [api:Form::setStrictFormMethodCheck()]." The same advice is noted in [Form Security](https://github.com/silverstripe/silverstripe-framework/blob/c2292a4cc1649448b1eac8b651dba03466da8be5/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md#strict-form-submission). Why not make this the default behaviour? Is there a scenario where this would cause a problem? Have manually tested in the CMS (alpha7) and is working fine. Note: Original commit that establised the API Form::setStrictFormMethodCheck is 14c59be8. --- docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md | 8 ++++---- src/Forms/Form.php | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md b/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md index d3947241d..94ca3a4b5 100644 --- a/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md +++ b/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md @@ -47,18 +47,18 @@ application up to `CSRF` security holes. ## Strict Form Submission -Forms should be limited to the intended HTTP verb (mostly `GET` or `POST`) to further reduce attack exposure. Without +To reduce attack exposure forms are limited, by default, to the intended HTTP verb (mostly `GET` or `POST`). Without this check, forms that rely on `GET` can be submitted via `POST` or `PUT` or vice-versa potentially leading to -application errors or edge cases. +application errors or edge cases. If you need to disable this setting follow the below example: :::php $form = new Form(..); $form->setFormMethod('POST'); - $form->setStrictFormMethodCheck(true); + $form->setStrictFormMethodCheck(false); // or alternative short notation.. - $form->setFormMethod('POST', true); + $form->setFormMethod('POST', false); ## Spam and Bot Attacks diff --git a/src/Forms/Form.php b/src/Forms/Form.php index edf4895c3..6694264ac 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -128,7 +128,7 @@ class Form extends ViewableData implements HasRequestHandler /** * @var boolean */ - protected $strictFormMethodCheck = false; + protected $strictFormMethodCheck = true; /** * Populated by {@link loadDataFrom()}. @@ -1036,13 +1036,13 @@ class Form extends ViewableData implements HasRequestHandler } /** - * If set to true, enforce the matching of the form method. + * If set to true (the default), enforces the matching of the form method. * * This will mean two things: * - GET vars will be ignored by a POST form, and vice versa * - A submission where the HTTP method used doesn't match the form will return a 400 error. * - * If set to false (the default), then the form method is only used to construct the default + * If set to false then the form method is only used to construct the default * form. * * @param $bool boolean