This was originally reported as a private security issue, but has been approved for handling in the public queue by the Drupal Security Team.

Background information

Problem/Motivation

The locale.check_translation route empties the locale project storage without a confirmation.
The impact is very limited but this feels like something that should be protected against CSRF.

Steps to reproduce

  1. As a user that can insert img tag on the site, insert this HTML:
    <img src="http://example.com/admin/reports/translations/check">
  2. If a user with the "translate interface" permission displays the page, the locale project storage is emptied.

Proposed resolution

Add the _csrf_token requirement to the route.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3575821

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

prudloff created an issue. See original summary.

prudloff’s picture

prudloff commented
Issue summary: View changes

prudloff opened merge request !14913

prudloff’s picture

prudloff commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Left comments on the MR.

prudloff’s picture

prudloff commented
Status: Needs work » Needs review

Comments have been addressed.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

I think this looks OK now.

  • longwave committed b3ddfca1 on 11.4.x 
    fix: #3575821 locale.check_translation route is not protected against...

  • longwave committed 4b42b8c5 on 11.x 
    fix: #3575821 locale.check_translation route is not protected against...

  • longwave committed 474d7c36 on main 
    fix: #3575821 locale.check_translation route is not protected against...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 10.6.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed and pushed 474d7c361e8 to main and 4b42b8c533a to 11.x and b3ddfca1463 to 11.4.x. Thanks!

As a security fix this is also eligible for backport to 10.6.x but it doesn't cherry-pick cleanly; leaving open for backport if someone can open a new MR against 10.6.x.

smustgrave opened merge request !16679

smustgrave’s picture

smustgrave commented
Status: Patch (to be ported) » Reviewed & tested by the community

Since it's been 1 month don't want this one to get forgotten

Moving to RTBC as the backport was pretty straight forward just issue around 

   - $this->drupalGet('admin/reports/translations/check');
   + $this->drupalGet('admin/reports/translations');
   + $this->clickLink('Check manually');
   +  $this->checkForMetaRefresh();
smustgrave’s picture

smustgrave commented
Status: Reviewed & tested by the community » Needs work

Oops forgot something one second.

smustgrave’s picture

smustgrave commented
Status: Needs work » Reviewed & tested by the community

Oh no I didn't core/modules/locale/tests/src/Functional/LocaleFileHashTest.php isn't in 10.6.x

  • longwave committed 1f5f2c4e on 10.6.x 
    fix: #3575821 locale.check_translation route is not protected against...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Let's just close this out.

Committed and pushed 1f5f2c4ee46 to 10.6.x. Thanks!

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.