Add security hardening protection option for programmatic form submission

Here is a forward-port to D8 with some basic tests. Stills needs the documentation paragraph explaining the existence and usage of programmed_bypass_access_check in FormBuilderInterface::submitForm().

Overall this looks very good and we really need this in D8.

1. +++ b/core/lib/Drupal/Core/Form/FormBuilderInterface.php
   @@ -158,6 +158,12 @@ public function getForm($form_arg);
   +   *   - programmed_bypass_access_check: If TRUE, programmatic form submissions
   +   *     are processed without taking #access into account. Set this to FALSE
   +   *     when submitting a form programmatically with values that may have been
   +   *     input by the user executing the current request; this will cause
   +   *     #access to be respected as it would on a normal form submission.
   +   *     Defaults to TRUE.
   

   Great documentation!

2. +++ b/core/modules/system/lib/Drupal/system/Tests/Form/ProgrammaticTest.php
   @@ -99,4 +99,31 @@ private function submitForm($values, $valid_input) {
   +    $this->assertTrue(isset($values['field_restricted']) && $values['field_restricted'] == 'dummy value', 'The value for the restricted field is stored correctly.');
   

   Using isset() to avoid notices being thrown is ingrained in every PHP developer's muscle memory, but has no place in a test. Use $this->assertEqual() instead to compare the two values.

3. +++ b/core/modules/system/lib/Drupal/system/Tests/Form/ProgrammaticTest.php
   @@ -99,4 +99,31 @@ private function submitForm($values, $valid_input) {
   +    $this->assertTrue(!isset($values['field_restricted']) || $values['field_restricted'] != 'dummy value', 'The value for the restricted field is not stored.');
   +
   +  }
   

   This is actually testing two things, but which is the expected behaviour? I assume the value is expected to be empty, so you can use $this->assertFalse() on it.

   There is also a superfluous line of whitespace here.

Addressed the remarks from #5

It's looking great now, thanks!

I think we should default this to FALSE rather than TRUE. Can we open a follow-up for that maybe? It won't be in the backport to 6.x so don't want to hold this up.

• Replaced the calls to the deprecated drupal_form_submit() with \Drupal::formBuilder()->submitForm()
• Split off issue #2229617: Change form element access bypass default to FALSE on programmatic submissions. to change the default value.

Those changes look good, thanks for opening the follow-up.

Needs porting still I think.

Is already included in 7.x as a security fix in 7.26. Still needs to be backported to 6.x.

We try to get the D6 version of the important services module running again and we have a security issue waiting of D6 port of this patch: #2189921: D6 Backport of "SA-CONTRIB-2014-007 - Services - Multiple access bypass vulnerabilities"

The test of D6 Backport patch by @David_Rothstein (#1) is passing. Needs some review now.

The patch in #1 works fine and provides the necessary hardening.

Thanks @JamesK for your review.

Can we please get this patch committed to D6?