Security backdrop#179: Tighten image style derivative access checks.

Author: quicksketch

core/includes/stream_wrappers.inc

@@ -932,6 +932,23 @@ class BackdropPublicStreamWrapper extends BackdropLocalStreamWrapper {
     $path = str_replace('\\', '/', $this->getTarget());
     return $GLOBALS['base_url'] . '/' . self::getDirectoryPath() . '/' . backdrop_encode_path($path);
   }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getLocalPath($uri = NULL) {
+    $path = parent::getLocalPath($uri);
+
+    $private_path = config_get('system.core', 'file_private_path');
+    if ($private_path) {
+      $private_path = realpath($private_path);
+      if ($private_path && strpos($path, $private_path) === 0) {
+        return FALSE;
+      }
+    }
+
+    return $path;
+  }
 }
 
 /**

core/modules/image/image.module

@@ -702,6 +702,7 @@ function image_style_deliver($style, $scheme) {
 
   $image_uri = $scheme . '://' . $target;
   $derivative_uri = image_style_path($style['name'], $image_uri);
+  $derivative_scheme = file_uri_scheme($derivative_uri);
 
   // Image styles are not allowed to modify previously generated style images.
   if (strpos($image_uri, $scheme . '://styles/') === 0) {
@@ -711,21 +712,25 @@ function image_style_deliver($style, $scheme) {
     backdrop_exit();
   }
 
-  // If using the private scheme, let other modules provide headers and
-  // control access to the file.
-  if ($scheme == 'private') {
-    if (file_exists($derivative_uri)) {
-      file_download($scheme, file_uri_target($derivative_uri));
+  $core_schemes = array('public', 'private', 'temporary');
+  $additional_public_schemes = array_diff((array) config_get('system.core', 'file_additional_public_schemes'), $core_schemes);
+  $public_schemes = array_merge(array('public'), $additional_public_schemes);
+  $is_public = in_array($derivative_scheme, $public_schemes, TRUE);
+
+  if ($scheme == 'private' && file_exists($derivative_uri)) {
+    file_download($scheme, file_uri_target($derivative_uri));
+  }
+
+  // Other non-public schemes may add additional headers and control access to
+  // the file.
+  if (!$is_public) {
+    $headers = file_download_headers($image_uri);
+    if (empty($headers)) {
+      return MENU_ACCESS_DENIED;
     }
-    else {
-      $headers = file_download_headers($image_uri);
-      if (empty($headers)) {
-        return MENU_ACCESS_DENIED;
-      }
-      if (count($headers)) {
-        foreach ($headers as $name => $value) {
-          backdrop_add_http_header($name, $value);
-        }
+    if (count($headers)) {
+      foreach ($headers as $name => $value) {
+        backdrop_add_http_header($name, $value);
       }
     }
   }
@@ -745,9 +750,12 @@ function image_style_deliver($style, $scheme) {
   }
 
   // Confirm that the original source image exists before trying to process it.
-  if (!is_file($image_uri)) {
+  if (!_image_source_image_exists($image_uri)) {
     watchdog('image', 'Source image at %source_image_path not found while trying to generate derivative image at %derivative_path.',  array('%source_image_path' => $image_uri, '%derivative_path' => $derivative_uri));
-    return MENU_NOT_FOUND;
+    backdrop_add_http_header('Status', '404 Not Found');
+    backdrop_add_http_header('Content-Type', 'text/html; charset=utf-8');
+    print t('Source image not found.');
+    backdrop_exit();
   }
 
   // Don't start generating the image if the derivative already exists or if
@@ -793,6 +801,39 @@ function image_style_deliver($style, $scheme) {
   }
 }
 
+/**
+ * Checks whether the provided source image exists.
+ *
+ * @param string $image_uri
+ *   The URI for the source image.
+ *
+ * @return bool
+ *   Whether the source image exists.
+ */
+function _image_source_image_exists($image_uri) {
+  $exists = file_exists($image_uri);
+
+  // If the file doesn't exist, we can stop here.
+  if (!$exists) {
+    return FALSE;
+  }
+
+  if (file_uri_scheme($image_uri) !== 'public') {
+    return TRUE;
+  }
+
+  $image_path = backdrop_realpath($image_uri);
+  $private_path = config_get('system.core', 'file_private_path');
+  if ($private_path) {
+    $private_path = realpath($private_path);
+    if ($private_path && strpos($image_path, $private_path) === 0) {
+      return FALSE;
+    }
+  }
+
+  return TRUE;
+}
+
 /**
  * Creates a new image derivative based on an image style.
  *

core/modules/system/config/system.core.json

@@ -24,6 +24,7 @@
     "default_nodes_main": 10,
     "error_level": "hide",
     "field_purge_batch_size": 200,
+    "file_additional_public_schemes": [],
     "file_default_allowed_extensions": "jpg jpeg gif png txt doc docx xls xlsx pdf ppt pptx pps ppsx odt ods odp mp3 mov mp4 m4a m4v mpeg avi ogg oga ogv weba webp webm",
     "file_default_scheme": "public",
     "file_private_path": "",

core/modules/system/system.install

@@ -2477,6 +2477,7 @@ function system_update_1025() {
  */
 function system_update_1026() {
   $config = config('system.core');
+  $config->set('file_additional_public_schemes', update_variable_get('file_additional_public_schemes', array()));
   $config->set('file_default_scheme', update_variable_get('file_default_scheme', 'public'));
   $config->set('file_public_path', update_variable_get('file_public_path', 'files'));
   $config->set('file_temporary_path', update_variable_get('file_temporary_path', ''));
@@ -3304,6 +3305,17 @@ function system_update_1082() {
   }
 }
 
+/**
+ * Set the default value for "file_additional_public_schemes" setting.
+ */
+function system_update_1083() {
+  $config = config('system.core');
+  if ($config->get('file_additional_public_schemes') == NULL) {
+    $config->set('file_additional_public_schemes', array());
+    $config->save();
+  }
+}
+
 /**
  * @} End of "defgroup updates-7.x-to-1.x"
  * The next series of updates should start at 2000.

core/modules/system/system.module

@@ -4388,6 +4388,24 @@ function system_admin_paths() {
   return $paths;
 }
 
+/**
+ * Implements hook_file_download().
+ */
+function system_file_download($uri) {
+  $core_schemes = array('public', 'private', 'temporary');
+  $additional_public_schemes = array_diff((array) config_get('system.core', 'file_additional_public_schemes'), $core_schemes);
+  if ($additional_public_schemes) {
+    $scheme = file_uri_scheme($uri);
+    if (in_array($scheme, $additional_public_schemes, TRUE)) {
+      return array(
+        // Returning any header grants access, and setting the 'Cache-Control'
+        // header is appropriate for public files.
+        'Cache-Control' => 'public',
+      );
+    }
+  }
+}
+
 /**
  * Sets up a form to save information automatically.
  *

core/modules/views/includes/utility.inc

@@ -511,7 +511,7 @@ function views_discover_plugins() {
  * Get enabled display extenders.
  */
 function views_get_enabled_display_extenders() {
-  $enabled = array_filter(config_get('views.settings', 'display_extenders'));
+  $enabled = array_filter((array) config_get('views.settings', 'display_extenders'));
   $options = views_fetch_plugin_names('display_extender');
   foreach ($options as $name => $plugin) {
     $enabled[$name] = $name;

settings.php

@@ -455,6 +455,29 @@
  */
 //$config['system.core']['block_interest_cohort'] = FALSE;
 
+/**
+ * Additional public file schemes:
+ *
+ * Public schemes are URI schemes that allow download access to all users for
+ * all files within that scheme.
+ *
+ * The "public" scheme is always public, and the "private" scheme is always
+ * private, but other schemes, such as "https", "s3", "example", or others,
+ * can be either public or private depending on the site. By default, they're
+ * private, and access to individual files is controlled via
+ * hook_file_download().
+ *
+ * Typically, if a scheme should be public, a module makes it public by
+ * implementing hook_file_download(), and granting access to all users for all
+ * files. This could be either the same module that provides the stream wrapper
+ * for the scheme, or a different module that decides to make the scheme
+ * public. However, in cases where a site needs to make a scheme public, but
+ * is unable to add code in a module to do so, the scheme may be added to this
+ * variable, the result of which is that system_file_download() grants public
+ * access to all files within that scheme.
+ */
+//$config['system.core']['file_additional_public_schemes'] = array('example');
+
 /**
  * Include a local settings file, if available.
  *