From 357efb6c33771a522146fac77f0939f409bb1bf2 Mon Sep 17 00:00:00 2001 From: xjm <xjm@65776.no-reply.drupal.org> Date: Wed, 17 Jun 2020 12:56:11 -0500 Subject: [PATCH] SA-CORE-2020-004 by samuel.mortenson, DorTumarkin, greggles, xjm, larowlan, webchick, pwolanin, dawehner, mcdruid, alexpott, dsnopek (cherry picked from commit 3999b8f658bf2ef8e96a7ee8ccb279c5d3073006) --- core/lib/Drupal/Core/Form/FormBuilder.php | 11 +++++- core/lib/Drupal/Core/Form/FormValidator.php | 4 +- .../Functional/FileManagedFileElementTest.php | 2 +- .../tests/src/Functional/Form/FormTest.php | 38 +++++++++++-------- .../src/Functional/Form/ValidationTest.php | 2 +- .../Tests/Core/Form/FormBuilderTest.php | 20 +++++++++- .../Drupal/Tests/Core/Form/FormTestBase.php | 2 +- .../Tests/Core/Form/FormValidatorTest.php | 2 +- 8 files changed, 57 insertions(+), 24 deletions(-) diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index ae2851ed221c..5e8e22a60422 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -19,6 +19,7 @@ use Drupal\Core\Theme\ThemeManagerInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\FileBag; +use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Response; @@ -957,8 +958,16 @@ public function doBuildForm($form_id, &$element, FormStateInterface &$form_state // This value is checked in self::handleInputElement(). $form_state->setInvalidToken(TRUE); + // Ignore all submitted values. + $form_state->setUserInput([]); + + $request = $this->requestStack->getCurrentRequest(); + // Do not trust any POST data. + $request->request = new ParameterBag(); // Make sure file uploads do not get processed. - $this->requestStack->getCurrentRequest()->files = new FileBag(); + $request->files = new FileBag(); + // Ensure PHP globals reflect these changes. + $request->overrideGlobals(); } } } diff --git a/core/lib/Drupal/Core/Form/FormValidator.php b/core/lib/Drupal/Core/Form/FormValidator.php index 57d24cc21e70..2afcb52ba9d6 100644 --- a/core/lib/Drupal/Core/Form/FormValidator.php +++ b/core/lib/Drupal/Core/Form/FormValidator.php @@ -124,10 +124,8 @@ public function validateForm($form_id, &$form, FormStateInterface &$form_state) * {@inheritdoc} */ public function setInvalidTokenError(FormStateInterface $form_state) { - $url = $this->requestStack->getCurrentRequest()->getRequestUri(); - // Setting this error will cause the form to fail validation. - $form_state->setErrorByName('form_token', $this->t('The form has become outdated. Copy any unsaved work in the form below and then <a href=":link">reload this page</a>.', [':link' => $url])); + $form_state->setErrorByName('form_token', $this->t('The form has become outdated. Press the back button, copy any unsaved work in the form, and then reload the page.')); } /** diff --git a/core/modules/file/tests/src/Functional/FileManagedFileElementTest.php b/core/modules/file/tests/src/Functional/FileManagedFileElementTest.php index 348cee7d3a99..3f67a7948a84 100644 --- a/core/modules/file/tests/src/Functional/FileManagedFileElementTest.php +++ b/core/modules/file/tests/src/Functional/FileManagedFileElementTest.php @@ -50,7 +50,7 @@ public function testManagedFile() { $file_field_name => \Drupal::service('file_system')->realpath($test_file->getFileUri()), ]; $this->drupalPostForm(NULL, $edit, t('Save')); - $this->assertText('The form has become outdated. Copy any unsaved work in the form below'); + $this->assertText('The form has become outdated.'); $last_fid = $this->getLastFileId(); $this->assertEqual($last_fid_prior, $last_fid, 'File was not saved when uploaded with an invalid form token.'); diff --git a/core/modules/system/tests/src/Functional/Form/FormTest.php b/core/modules/system/tests/src/Functional/Form/FormTest.php index bf5fb2764645..39e304eee2f7 100644 --- a/core/modules/system/tests/src/Functional/Form/FormTest.php +++ b/core/modules/system/tests/src/Functional/Form/FormTest.php @@ -251,21 +251,27 @@ public function testInputWithInvalidToken() { $this->assertSession() ->elementExists('css', 'input[name="form_token"]') ->setValue('invalid token'); + $random_string = $this->randomString(); $edit = [ - 'textfield' => $this->randomString(), + 'textfield' => $random_string, 'checkboxes[bar]' => TRUE, 'select' => 'bar', 'radios' => 'foo', ]; $this->drupalPostForm(NULL, $edit, 'Submit'); $this->assertFieldByXpath('//div[contains(@class, "error")]', NULL, 'Error message is displayed with invalid token even when required fields are filled.'); - $this->assertText('The form has become outdated. Copy any unsaved work in the form below'); - // Verify that input elements retained the posted values. - $this->assertFieldByName('textfield', $edit['textfield']); + + $assert = $this->assertSession(); + $element = $assert->fieldExists('textfield'); + $this->assertEmpty($element->getValue()); + $assert->responseNotContains($random_string); + $this->assertText('The form has become outdated.'); + // Ensure that we don't use the posted values. + $this->assertFieldByName('textfield', ''); $this->assertNoFieldChecked('edit-checkboxes-foo'); - $this->assertFieldChecked('edit-checkboxes-bar'); - $this->assertOptionSelected('edit-select', 'bar'); - $this->assertFieldChecked('edit-radios-foo'); + $this->assertNoFieldChecked('edit-checkboxes-bar'); + $this->assertOptionSelected('edit-select', ''); + $this->assertNoFieldChecked('edit-radios-foo'); // Check another form that has a textarea input. $this->drupalGet(Url::fromRoute('form_test.required')); @@ -278,9 +284,9 @@ public function testInputWithInvalidToken() { ]; $this->drupalPostForm(NULL, $edit, 'Submit'); $this->assertFieldByXpath('//div[contains(@class, "error")]', NULL, 'Error message is displayed with invalid token even when required fields are filled.'); - $this->assertText('The form has become outdated. Copy any unsaved work in the form below'); - $this->assertFieldByName('textfield', $edit['textfield']); - $this->assertFieldByName('textarea', $edit['textarea']); + $this->assertText('The form has become outdated.'); + $this->assertFieldByName('textfield', ''); + $this->assertFieldByName('textarea', ''); // Check another form that has a number input. $this->drupalGet(Url::fromRoute('form_test.number')); @@ -288,12 +294,14 @@ public function testInputWithInvalidToken() { ->elementExists('css', 'input[name="form_token"]') ->setValue('invalid token'); $edit = [ - 'integer_step' => mt_rand(1, 100), + // We choose a random value which is higher than the default value, + // so we don't accidentally generate the default value. + 'integer_step' => mt_rand(6, 100), ]; $this->drupalPostForm(NULL, $edit, 'Submit'); $this->assertFieldByXpath('//div[contains(@class, "error")]', NULL, 'Error message is displayed with invalid token even when required fields are filled.'); - $this->assertText('The form has become outdated. Copy any unsaved work in the form below'); - $this->assertFieldByName('integer_step', $edit['integer_step']); + $this->assertText('The form has become outdated.'); + $this->assertFieldByName('integer_step', 5); // Check a form with a Url field $this->drupalGet(Url::fromRoute('form_test.url')); @@ -305,8 +313,8 @@ public function testInputWithInvalidToken() { ]; $this->drupalPostForm(NULL, $edit, 'Submit'); $this->assertFieldByXpath('//div[contains(@class, "error")]', NULL, 'Error message is displayed with invalid token even when required fields are filled.'); - $this->assertText('The form has become outdated. Copy any unsaved work in the form below'); - $this->assertFieldByName('url', $edit['url']); + $this->assertText('The form has become outdated.'); + $this->assertFieldByName('url', ''); } /** diff --git a/core/modules/system/tests/src/Functional/Form/ValidationTest.php b/core/modules/system/tests/src/Functional/Form/ValidationTest.php index 3ae3ffee1ac6..9834eef8bb60 100644 --- a/core/modules/system/tests/src/Functional/Form/ValidationTest.php +++ b/core/modules/system/tests/src/Functional/Form/ValidationTest.php @@ -74,7 +74,7 @@ public function testValidate() { $this->drupalPostForm(NULL, ['name' => 'validate'], 'Save'); $this->assertNoFieldByName('name', '#value changed by #validate', 'Form element #value was not altered.'); $this->assertNoText('Name value: value changed by setValueForElement() in #validate', 'Form element value in $form_state was not altered.'); - $this->assertText('The form has become outdated. Copy any unsaved work in the form below'); + $this->assertText('The form has become outdated.'); } /** diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index 2b3de5de53d1..8060341d32e8 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -832,12 +832,30 @@ public function testInvalidToken($expected, $valid_token, $user_is_authenticated $expected_form = $form_id(); $form_arg = $this->getMockForm($form_id, $expected_form); + // Set up some request data so we can be sure it is removed when a token is + // invalid. + $this->request->request->set('foo', 'bar'); + $_POST['foo'] = 'bar'; + $form_state = new FormState(); $input['form_id'] = $form_id; $input['form_token'] = $form_token; + $input['test'] = 'example-value'; $form_state->setUserInput($input); - $this->simulateFormSubmission($form_id, $form_arg, $form_state, FALSE); + $form = $this->simulateFormSubmission($form_id, $form_arg, $form_state, FALSE); $this->assertSame($expected, $form_state->hasInvalidToken()); + if ($expected) { + $this->assertEmpty($form['test']['#value']); + $this->assertEmpty($form_state->getValue('test')); + $this->assertEmpty($_POST); + $this->assertEmpty(iterator_to_array($this->request->request->getIterator())); + } + else { + $this->assertEquals('example-value', $form['test']['#value']); + $this->assertEquals('example-value', $form_state->getValue('test')); + $this->assertEquals('bar', $_POST['foo']); + $this->assertEquals('bar', $this->request->request->get('foo')); + } } public function providerTestInvalidToken() { diff --git a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php index 5e4b64c4dd8d..56264ece281c 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php +++ b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php @@ -173,7 +173,7 @@ protected function setUp() { ->getMock(); $this->account = $this->createMock('Drupal\Core\Session\AccountInterface'); $this->themeManager = $this->createMock('Drupal\Core\Theme\ThemeManagerInterface'); - $this->request = new Request(); + $this->request = Request::createFromGlobals(); $this->eventDispatcher = $this->createMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); $this->requestStack = new RequestStack(); $this->requestStack->push($this->request); diff --git a/core/tests/Drupal/Tests/Core/Form/FormValidatorTest.php b/core/tests/Drupal/Tests/Core/Form/FormValidatorTest.php index 9b1b73ef3383..7eb5524a6904 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormValidatorTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormValidatorTest.php @@ -131,7 +131,7 @@ public function testValidateInvalidFormToken() { ->getMock(); $form_state->expects($this->once()) ->method('setErrorByName') - ->with('form_token', 'The form has become outdated. Copy any unsaved work in the form below and then <a href="/test/example?foo=bar">reload this page</a>.'); + ->with('form_token', 'The form has become outdated. Press the back button, copy any unsaved work in the form, and then reload the page.'); $form_state->setValue('form_token', 'some_random_token'); $form_validator->validateForm('test_form_id', $form, $form_state); $this->assertTrue($form_state->isValidationComplete()); -- GitLab