diff --git a/core/modules/image/src/Controller/ImageStyleDownloadController.php b/core/modules/image/src/Controller/ImageStyleDownloadController.php index 2b2d89f..d90f52e 100644 --- a/core/modules/image/src/Controller/ImageStyleDownloadController.php +++ b/core/modules/image/src/Controller/ImageStyleDownloadController.php @@ -102,9 +102,14 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st // generated without a token can set the // 'image.settings:allow_insecure_derivatives' configuration to TRUE to // bypass the latter check, but this will increase the site's vulnerability - // to denial-of-service attacks. + // site's vulnerability to denial-of-service attacks. To prevent this + // variable from leaving the site vulnerable to the most serious attacks, a + // token is always required when a derivative of a style is requested. + // The $target variable for a derivative of a style has + // styles//... as structure, so we check if the $target variable + // starts with styles/. $valid = !empty($image_style) && file_stream_wrapper_valid_scheme($scheme); - if (!$this->config('image.settings')->get('allow_insecure_derivatives')) { + if (!$this->config('image.settings')->get('allow_insecure_derivatives') || strpos(ltrim($target, '\/'), 'styles/') === 0) { $valid &= $request->query->get(IMAGE_DERIVATIVE_TOKEN) === $image_style->getPathToken($image_uri); } if (!$valid) { diff --git a/core/modules/image/src/Entity/ImageStyle.php b/core/modules/image/src/Entity/ImageStyle.php index 2ba7a4f..7d97d59 100644 --- a/core/modules/image/src/Entity/ImageStyle.php +++ b/core/modules/image/src/Entity/ImageStyle.php @@ -276,6 +276,13 @@ public function flush($path = NULL) { * {@inheritdoc} */ public function createDerivative($original_uri, $derivative_uri) { + + // If the source file doesn't exist, return FALSE without creating folders. + $image = \Drupal::service('image.factory')->get($original_uri); + if (!$image->isValid()) { + return FALSE; + } + // Get the folder for the final location of this style. $directory = drupal_dirname($derivative_uri); @@ -285,11 +292,6 @@ public function createDerivative($original_uri, $derivative_uri) { return FALSE; } - $image = \Drupal::service('image.factory')->get($original_uri); - if (!$image->isValid()) { - return FALSE; - } - foreach ($this->getEffects() as $effect) { $effect->applyEffect($image); } diff --git a/core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php b/core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php index eb09e07..de107d1 100644 --- a/core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php +++ b/core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php @@ -224,6 +224,33 @@ function doImageStyleUrlAndPathTests($scheme, $clean_url = TRUE, $extra_slash = $this->assertIdentical(strpos($generate_url, IMAGE_DERIVATIVE_TOKEN . '='), FALSE, 'The security token does not appear in the image style URL.'); $this->drupalGet($generate_url); $this->assertResponse(200, 'Image was accessible at the URL with a missing token.'); + + // Enable the security token again + $this->config('image.settings')->set('suppress_itok_output', FALSE)->save(); + // Check that a security token is still required when generating a second + // image derivative using the first one as a source. + $nested_url = $this->style->buildUrl($generated_uri, $clean_url); + $this->pass($nested_url); + $this->verbose($nested_url); + $nested_url_with_wrong_token = str_replace(IMAGE_DERIVATIVE_TOKEN . '=', 'wrongparam=', $nested_url); + $this->drupalGet($nested_url_with_wrong_token); + $this->assertResponse(403, 'Image generated from an earlier derivative was inaccessible at the URL with a missing token.'); + // Check that this restriction cannot be bypassed by adding extra slashes + // to the URL. + $this->drupalGet(substr_replace($nested_url_with_wrong_token, '//styles/', strrpos($nested_url_with_wrong_token, '/styles/'), strlen('/styles/'))); + $this->assertResponse(403, 'Image generated from an earlier derivative was inaccessible at the URL with a missing token, even with an extra forward slash in the URL.'); + $this->drupalGet(substr_replace($nested_url_with_wrong_token, '/\styles/', strrpos($nested_url_with_wrong_token, '/styles/'), strlen('/styles/'))); + $this->verbose(substr_replace($nested_url_with_wrong_token, '/\styles/', strrpos($nested_url_with_wrong_token, '/styles/'), strlen('/styles/'))); + $this->assertResponse(403, 'Image generated from an earlier derivative was inaccessible at the URL with a missing token, even with an extra backslash in the URL.'); + // Make sure the image can still be generated if a correct token is used. + $this->drupalGet($nested_url); + $this->assertResponse(200, 'Image was accessible when a correct token was provided in the URL.'); + + // Check that requesting a nonexistent image does not create any new + // directories in the file system. + $directory = $scheme . '://styles/' . $this->style->id() . '/' . $scheme . '/' . $this->randomString(); + $this->drupalGet(file_create_url($directory . '/' . $this->randomString())); + $this->assertFalse(file_exists($directory), 'New directory was not created in the filesystem when requesting an unauthorized image.'); } }