$http_host will contain a . or two, which is a special character for regexps and should be quoted. This would also remove the slim possibility of using this as part of a ReDoS attack.

CommentFileSizeAuthor
#14 drupal-file_url_transform_relative-2257231-14.patch2.62 KBsebastien m.
#12 drupal-file_url_transform_relative-2257231-12.patch2.59 KBsebastien m.
#3 2257231.diff438 bytesdrumm
#1 2257231.patch433 bytesdrumm

Comments

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Active » Needs review
StatusFileSize
new 2257231.patch433 bytes
drumm’s picture

drumm
he/him
Location NY, US
commented
Issue summary: View changes

(fix typo in issue summary)

drumm’s picture

drumm
he/him
Location NY, US
commented
StatusFileSize
new 2257231.diff438 bytes

Adding the delimiter to preg_quote().

Status: Needs review » Needs work

The last submitted patch, 3: 2257231.diff, failed testing.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs work » Needs review

That seems unreasonable.

drumm’s picture

drumm
he/him
Location NY, US
commented

3: 2257231.diff queued for re-testing.

meeli’s picture

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

Seems reasonable, but it seems like we need some tests for this to make sure it outputs what we're expecting.

drumm’s picture

drumm
he/him
Location NY, US
commented
Assigned: drumm » Unassigned

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.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.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.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.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

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

sebastien m.’s picture

sebastien m. commented
Status: Needs work » Needs review
StatusFileSize
new drupal-file_url_transform_relative-2257231-12.patch2.59 KB

This patch includes kernel tests

Status: Needs review » Needs work

The last submitted patch, 12: drupal-file_url_transform_relative-2257231-12.patch, failed testing.

sebastien m.’s picture

sebastien m. commented
StatusFileSize
new drupal-file_url_transform_relative-2257231-14.patch2.62 KB

my fault, file paths updated.

sebastien m.’s picture

sebastien m. commented
Status: Needs work » Needs review

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.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.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

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

jody lynn’s picture

jody lynn
she/her
Primary language English
commented

I actually just got bit by this issue.

I'm serving files from s3 with URLs like https://foo-org-private.s3.amazonaws.com/.... on the site foo.org. So the result of file_url_transform_relative (which was called by FileMediaFormatterBase) was to change my URLs to be -private.s3.amazonaws.com/....

Not only did it mix up the -org with .org as described here, but it also didn't mind that my domain name only began with my host domain rather than matching it.

jody lynn’s picture

jody lynn
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Confirmed the patch fixes it.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Category: Task » Bug report
Priority: Minor » Normal
Issue tags: -Needs tests

I've confirmed the tests fail without the fix.

Testing Drupal\KernelTests\Core\File\UrlTransformRelativeTest
....F

Time: 2.82 seconds, Memory: 6.00MB

There was 1 failure:

1) Drupal\KernelTests\Core\File\UrlTransformRelativeTest::testFileUrlTransformRelative with data set #4 ('example.com', 80, '', 'http://exampleXcom/page', 'http://exampleXcom/page')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-http://exampleXcom/page
+/page

/Volumes/devdisk/dev/sites/drupal8alt.dev/core/tests/Drupal/KernelTests/Core/File/UrlTransformRelativeTest.php:40

FAILURES!
Tests: 5, Assertions: 9, Failures: 1.
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Patch (to be ported)

Crediting @Jody Lynn for testing that fix covers the scenario detailed in #18.

Committed d32d0c5 and pushed to 8.6.x. Thanks!

Setting to patch to be ported to backport to 8.5.x once the commit freeze is over.

  • alexpott committed d32d0c5 on 8.6.x 
    Issue #2257231 by drumm, Sebastien @Actualys, Jody Lynn:...

  • alexpott committed 0a030d9 on 8.5.x 
    Issue #2257231 by drumm, Sebastien @Actualys, Jody Lynn:...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.