Merge branch '7.20-hotfix' into 7.x

Conflicts:
	CHANGELOG.txt
	includes/bootstrap.inc

CHANGELOG.txt

@@ -1,5 +1,5 @@
 
-Drupal 7.21, xxxx-xx-xx (development version)
+Drupal 7.22, xxxx-xx-xx (development version)
 -----------------------
 - Fixed a bug which caused the denial-of-service protection added in Drupal
   7.20 to break certain valid image URLs that had an extra slash in them.
@@ -32,6 +32,11 @@ Drupal 7.21, xxxx-xx-xx (development version)
   sites which use HTTPS and redirect between "www" and non-"www" versions of
   the page.
 
+Drupal 7.21, 2013-03-06
+-----------------------
+- Allowed sites using the 'image_allow_insecure_derivatives' variable to still
+  have partial protection from the security issues fixed in Drupal 7.20.
+
 Drupal 7.20, 2013-02-20
 -----------------------
 - Fixed security issues (denial of service). See SA-CORE-2013-002.

includes/bootstrap.inc

@@ -8,7 +8,7 @@
 /**
  * The current system version.
  */
-define('VERSION', '7.21-dev');
+define('VERSION', '7.22-dev');
 
 /**
  * Core API compatibility.

modules/image/image.module

@@ -796,9 +796,11 @@ function image_style_deliver($style, $scheme) {
   // derivative token is valid. (Sites which require image derivatives to be
   // generated without a token can set the 'image_allow_insecure_derivatives'
   // variable 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 derivative is requested.)
   $valid = !empty($style) && file_stream_wrapper_valid_scheme($scheme);
-  if (!variable_get('image_allow_insecure_derivatives', FALSE)) {
+  if (!variable_get('image_allow_insecure_derivatives', FALSE) || strpos(ltrim($target, '\/'), 'styles/') === 0) {
     $valid = $valid && isset($_GET[IMAGE_DERIVATIVE_TOKEN]) && $_GET[IMAGE_DERIVATIVE_TOKEN] === image_style_path_token($style['name'], $scheme . '://' . $target);
   }
   if (!$valid) {
@@ -883,6 +885,11 @@ function image_style_deliver($style, $scheme) {
  * @see image_style_load()
  */
 function image_style_create_derivative($style, $source, $destination) {
+  // If the source file doesn't exist, return FALSE without creating folders.
+  if (!$image = image_load($source)) {
+    return FALSE;
+  }
+
   // Get the folder for the final location of this style.
   $directory = drupal_dirname($destination);
 
@@ -892,10 +899,6 @@ function image_style_create_derivative($style, $source, $destination) {
     return FALSE;
   }
 
-  if (!$image = image_load($source)) {
-    return FALSE;
-  }
-
   foreach ($style['effects'] as $effect) {
     image_effect_apply($image, $effect);
   }

modules/image/image.test

@@ -271,6 +271,51 @@ class ImageStylesPathAndUrlTestCase extends DrupalWebTestCase {
       $this->drupalGet(str_replace(IMAGE_DERIVATIVE_TOKEN . '=', IMAGE_DERIVATIVE_TOKEN . '=Zo', $generate_url));
       $this->assertResponse(200, 'Existing image was accessible at the URL wih an invalid token.');
     }
+
+    // Allow insecure image derivatives to be created for the remainder of this
+    // test.
+    variable_set('image_allow_insecure_derivatives', TRUE);
+
+    // Create another working copy of the file.
+    $files = $this->drupalGetTestFiles('image');
+    $file = array_shift($files);
+    $image_info = image_get_info($file->uri);
+    $original_uri = file_unmanaged_copy($file->uri, $scheme . '://', FILE_EXISTS_RENAME);
+    // Let the image_module_test module know about this file, so it can claim
+    // ownership in hook_file_download().
+    variable_set('image_module_test_file_download', $original_uri);
+
+    // Get the URL of a file that has not been generated and try to create it.
+    $generated_uri = image_style_path($this->style_name, $original_uri);
+    $this->assertFalse(file_exists($generated_uri), 'Generated file does not exist.');
+    $generate_url = image_style_url($this->style_name, $original_uri);
+
+    // Check that the image is accessible even without the security token.
+    $this->drupalGet(str_replace(IMAGE_DERIVATIVE_TOKEN . '=', 'wrongparam=', $generate_url));
+    $this->assertResponse(200, 'Image was accessible at the URL with a missing token.');
+
+    // Check that a security token is still required when generating a second
+    // image derivative using the first one as a source.
+    $nested_uri = image_style_path($this->style_name, $generated_uri);
+    $nested_url = image_style_url($this->style_name, $generated_uri);
+    $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 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_name . '/' . $scheme . '/' . $this->randomName();
+    $this->drupalGet(file_create_url($directory . '/' . $this->randomName()));
+    $this->assertFalse(file_exists($directory), 'New directory was not created in the filesystem when requesting an unauthorized image.');
   }
 }