Commit b37e84f0 authored by alexpott's avatar alexpott

Issue #1934568 by David_Rothstein, StryKaizer, borisson_, martin107, pwolanin,...

Issue #1934568 by David_Rothstein, StryKaizer, borisson_, martin107, pwolanin, ofry, amitgoyal: Allow sites using the 'image_allow_insecure_derivatives' variable to have partial protection from the Drupal 7.20 security issue
parent 0f8b5b1b
......@@ -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.
// 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/<style_name>/... 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) {
......
......@@ -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);
}
......
......@@ -224,6 +224,34 @@ 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.');
// Stop supressing the security token in the URL.
$this->config('image.settings')->set('suppress_itok_output', FALSE)->save();
// Ensure allow_insecure_derivatives is enabled.
$this->assertEqual($this->config('image.settings')->get('allow_insecure_derivatives'), TRUE);
// 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);
$matches_expected_url_format = (boolean) preg_match('/styles\/' . $this->style->id() . '\/' . $scheme . '\/styles\/' . $this->style->id() . '\/' . $scheme . '/', $nested_url);
$this->assertTrue($matches_expected_url_format, "Url for a derivative of an image style matches expected format.");
$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->assertResponse(403, 'Image generated from an earlier derivative was inaccessible at the URL with a missing token, even with multiple forward slashes 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->randomMachineName();
$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.');
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment