Currently we get constant PHP trim warnings because of automated attack scripts trying the SA-CORE-2018-002 vulnerability. The warning is

Warning: trim() expects parameter 1 to be string, array given in Drupal\Core\Render\Element\Email::validateEmail() (line 73 of /var/www/drupal/core/lib/Drupal/Core/Render/Element/Email.php) #0 /var/www/drupal/core/includes/bootstrap.inc(582): _drupal_error_handler_real(2, 'trim() expects ...', '/opt/rh/httpd24...', 73, Array) #1 [internal function]: _drupal_error_handler(2, 'trim() expects ...', '/opt/rh/httpd24...', 73, Array) #2 /var/www/drupal/core/lib/Drupal/Core/Render/Element/Email.php(73): trim(Array) #3 [internal function]: Drupal\Core\Render\Element\Email::validateEmail(Array, Object(Drupal\Core\Form\FormState), Array) #4 /var/www/drupal/core/lib/Drupal/Core/Form/FormValidator.php(283): call_user_func_array(Array, Array) #5 /var/www/drupal/core/lib/Drupal/Core/Form/FormValidator.php(239): Drupal\Core\Form\FormValidator->doValidateForm(Array, Object(Drupal\Core\Form\FormState)) #6 /var/www/drupal/core/lib/Drupal/Core/Form/FormValidator.php(239): Drupal\Core\Form\FormValidator->doValidateForm(Array, Object(Drupal\Core\Form\FormState)) #7 /var/www/drupal/core/lib/Drupal/Core/Form/FormValidator.php(119): Drupal\Core\Form\FormValidator->doValidateForm(Array, Object(Drupal\Core\Form\FormState), 'user_register_f...') #8 /var/www/drupal/core/lib/Drupal/Core/Form/FormBuilder.php(571): Drupal\Core\Form\FormValidator->validateForm('user_register_f...', Array, Object(Drupal\Core\Form\FormState)) #9 /var/www/drupal/core/lib/Drupal/Core/Form/FormBuilder.php(314): Drupal\Core\Form\FormBuilder->processForm('user_register_f...', Array, Object(Drupal\Core\Form\FormState)) #10 /var/www/drupal/core/lib/Drupal/Core/Controller/FormController.php(74): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\user\RegisterForm), Object(Drupal\Core\Form\FormState)) #11 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch)) #12 /var/www/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array) #13 /var/www/drupal/core/lib/Drupal/Core/Render/Renderer.php(582): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() #14 /var/www/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure)) #15 /var/www/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) #16 [internal function]: Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() #17 /var/www/drupal/vendor/symfony/http-kernel/HttpKernel.php(151): call_user_func_array(Object(Closure), Array) #18 /var/www/drupal/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1) #19 /var/www/drupal/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #20 /var/www/drupal/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #21 /var/www/drupal/core/modules/page_cache/src/StackMiddleware/PageCache.php(99): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #22 /var/www/drupal/core/modules/page_cache/src/StackMiddleware/PageCache.php(78): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true) #23 /var/www/drupal/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #24 /var/www/drupal/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(50): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #25 /var/www/drupal/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #26 /var/www/drupal/core/lib/Drupal/Core/DrupalKernel.php(664): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #27 /var/www/drupal/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request)) #28 {main}.

The attack scripts are trying to pass in an array into the trim($element['#value']).
Attached is a patch for the Drupal\Core\Render\Element\Email class.

Comments

dkarso created an issue. See original summary.

dkarso’s picture

Issue summary: View changes
dkarso’s picture

Status: Active » Needs review
dkarso’s picture

Assigned: dkarso » Unassigned
cilefen’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Can you modify a test to cover this? It actually seems difficult because if you try passing an array in a test such as Drupal\Tests\system\Functional\Form\EmailTest, dom-crawler doesn't like that for the same reason. Perhaps there could be a better place for that test. Also, it seems it would be an improvement to block this kind of activity for any string type field.

dkarso’s picture

Status: Needs work » Needs review
StatusFileSize
new4.11 KB

It seems there was no unit test for the Email render element. There were unit tests for Password and Textfield. I added an extra unit test for the Email render element in the second patch. The test catches the array notice error without the first patch. With the email patch everything looks ok.

Regarding your improvement suggestion: Drupal\Core\Render\Element\Textfield::valueCallback() already checks if the input is a scalar.

Status: Needs review » Needs work

The last submitted patch, 6: email-validateEmail-2.diff, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

dkarso’s picture

Status: Needs work » Needs review
StatusFileSize
new4.95 KB
cilefen’s picture

Issue tags: -Needs tests
StatusFileSize
new382 bytes

Please add interdiffs when making changes. I have attached it. Could you please post the tests-only patch to prove it fails for the expected reasons, and not for t() being out of scope? Usually, in one comment you can upload first the tests-only patch, then the complete patch. The tests-only patch will fail but the full patch will pass. Thank you.

dkarso’s picture

Test only patch and patch only patch added.

The last submitted patch, 10: email-validateEmail-test-only.diff, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

dkarso’s picture

As expected; the test-only patch catches the trim warning. The complete patch at #8 should do the trick.

dkarso’s picture

New complete patch file added and new test only diff with interdiffs. I had the test in the wrong group and missed a space character.

eric_a’s picture

I wonder if this would be better fixed in the form element type value callback by casting to string...

I scanned the test. Does the error count matter? If not, FormState::hasAnyErrors() would benefit clarity?

The last test-only is green... It contains both a test and a fix. It's identical to email-validateEmail-patch-only-2.diff. (Also, the interdiffs look pretty broken... @dkarso: https://www.drupal.org/documentation/git/interdiff)

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
super_romeo’s picture

Still in 9.3.10.

larowlan’s picture

yogeshmpawar’s picture

Updated patch will reroll diff.

Status: Needs review » Needs work

The last submitted patch, 22: 2973871-22.patch, failed testing. View results

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

rassoni’s picture

Status: Needs work » Needs review
StatusFileSize
new854 bytes
new2.36 KB

#22 patch applied successfully on d10. Fixed Test case failing .

rassoni’s picture

StatusFileSize
new3.36 KB
new605 bytes

#26missed file in patch.#22 patch applied successfully on d10. Fixed Test case failing .

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Ran the tests locally without the fix and they all pass without issue. So they will need to be updated.

Also the issue summary probably could be updated with steps to reproduce, proposed solution, etc.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

Status: Needs work » Postponed (maintainer needs more info)

#28 should of been moved to new status.

smustgrave’s picture

wanted to bump 1 more time before closing.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

smustgrave’s picture

Status: Postponed (maintainer needs more info) » Closed (outdated)

Since there's been no follow up and summary is pretty incomplete (missing steps) going to close out. But it can always be re-open if happening in D11+, if so please update the summary using the standard template

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.