From 0b4f0cda543bba0ebe2249bec051afe68443f91c Mon Sep 17 00:00:00 2001
From: xjm <xjm@65776.no-reply.drupal.org>
Date: Wed, 20 Jul 2022 10:11:30 -0500
Subject: [PATCH] SA-CORE-2022-012 by cmlara, GuyPaddock, larowlan, mondrake,
effulgentsia, xjm, longwave, Dave Reid, lauriii, David Strauss, benjifisher,
alexpott, mcdruid, Fabianx
(cherry picked from commit 1f82337d1789424dcb1901eb68b820a7e141d8c5)
---
.../scaffold/files/default.settings.php | 23 ++++++
.../Core/StreamWrapper/PublicStream.php | 24 ++++++
.../ImageStyleDownloadController.php | 82 ++++++++++++++++---
core/modules/system/system.module | 20 +++++
sites/default/default.settings.php | 23 ++++++
5 files changed, 159 insertions(+), 13 deletions(-)
diff --git a/core/assets/scaffold/files/default.settings.php b/core/assets/scaffold/files/default.settings.php
index 2b610fdf9aeb..447e94eef3b2 100644
--- a/core/assets/scaffold/files/default.settings.php
+++ b/core/assets/scaffold/files/default.settings.php
@@ -505,6 +505,29 @@
*/
# $settings['file_public_path'] = 'sites/default/files';
+/**
+ * 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.
+ */
+# $settings['file_additional_public_schemes'] = ['example'];
+
/**
* Private file path:
*
diff --git a/core/lib/Drupal/Core/StreamWrapper/PublicStream.php b/core/lib/Drupal/Core/StreamWrapper/PublicStream.php
index e89e43219eab..1747bf37c5f5 100644
--- a/core/lib/Drupal/Core/StreamWrapper/PublicStream.php
+++ b/core/lib/Drupal/Core/StreamWrapper/PublicStream.php
@@ -115,4 +115,28 @@ public static function basePath($site_path = NULL) {
return Settings::get('file_public_path', $site_path . '/files');
}
+ /**
+ * {@inheritdoc}
+ */
+ protected function getLocalPath($uri = NULL) {
+ $path = parent::getLocalPath($uri);
+ if (!$path || (strpos($path, 'vfs://') === 0)) {
+ return $path;
+ }
+
+ if (Settings::get('sa_core_2022_012_override') === TRUE) {
+ return $path;
+ }
+
+ $private_path = Settings::get('file_private_path');
+ if ($private_path) {
+ $private_path = realpath($private_path);
+ if ($private_path && strpos($path, $private_path) === 0) {
+ return FALSE;
+ }
+ }
+
+ return $path;
+ }
+
}
diff --git a/core/modules/image/src/Controller/ImageStyleDownloadController.php b/core/modules/image/src/Controller/ImageStyleDownloadController.php
index 8d56f89dfb96..775949dcb61b 100644
--- a/core/modules/image/src/Controller/ImageStyleDownloadController.php
+++ b/core/modules/image/src/Controller/ImageStyleDownloadController.php
@@ -6,6 +6,7 @@
use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\Image\ImageFactory;
use Drupal\Core\Lock\LockBackendInterface;
+use Drupal\Core\Site\Settings;
use Drupal\Core\StreamWrapper\StreamWrapperManager;
use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface;
use Drupal\image\ImageStyleInterface;
@@ -109,21 +110,25 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st
$target = $request->query->get('file');
$image_uri = $scheme . '://' . $target;
- // Check that the style is defined, the scheme is valid, and the image
- // derivative token is valid. Sites which require image derivatives to be
- // generated without a token can set the
+ // Check that the style is defined and the scheme is valid.
+ $valid = !empty($image_style) && $this->streamWrapperManager->isValidScheme($scheme);
+
+ // Also validate the derivative token. Sites which require image
+ // derivatives to be 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
+ // bypass this check, but this will increase the 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/<style_name>/... as structure, so we check if the $target variable
// starts with styles/.
- $valid = !empty($image_style) && $this->streamWrapperManager->isValidScheme($scheme);
+ $token = $request->query->get(IMAGE_DERIVATIVE_TOKEN, '');
+ $token_is_valid = hash_equals($image_style->getPathToken($image_uri), $token);
if (!$this->config('image.settings')->get('allow_insecure_derivatives') || strpos(ltrim($target, '\/'), 'styles/') === 0) {
- $valid &= hash_equals($image_style->getPathToken($image_uri), $request->query->get(IMAGE_DERIVATIVE_TOKEN, ''));
+ $valid = $valid && $token_is_valid;
}
+
if (!$valid) {
// Return a 404 (Page Not Found) rather than a 403 (Access Denied) as the
// image token is for DDoS protection rather than access checking. 404s
@@ -133,11 +138,23 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st
}
$derivative_uri = $image_style->buildUri($image_uri);
+ $derivative_scheme = $this->streamWrapperManager->getScheme($derivative_uri);
+
+ if ($token_is_valid) {
+ $is_public = ($scheme !== 'private');
+ }
+ else {
+ $core_schemes = ['public', 'private', 'temporary'];
+ $additional_public_schemes = array_diff(Settings::get('file_additional_public_schemes', []), $core_schemes);
+ $public_schemes = array_merge(['public'], $additional_public_schemes);
+ $is_public = in_array($derivative_scheme, $public_schemes, TRUE);
+ }
+
$headers = [];
- // If using the private scheme, let other modules provide headers and
+ // If not using a public scheme, let other modules provide headers and
// control access to the file.
- if ($scheme == 'private') {
+ if (!$is_public) {
$headers = $this->moduleHandler()->invokeAll('file_download', [$image_uri]);
if (in_array(-1, $headers) || empty($headers)) {
throw new AccessDeniedHttpException();
@@ -145,14 +162,14 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st
}
// Don't try to generate file if source is missing.
- if (!file_exists($image_uri)) {
+ if (!$this->sourceImageExists($image_uri, $token_is_valid)) {
// If the image style converted the extension, it has been added to the
// original file, resulting in filenames like image.png.jpeg. So to find
// the actual source image, we remove the extension and check if that
// image exists.
$path_info = pathinfo(StreamWrapperManager::getTarget($image_uri));
$converted_image_uri = sprintf('%s://%s%s%s', $this->streamWrapperManager->getScheme($derivative_uri), $path_info['dirname'], DIRECTORY_SEPARATOR, $path_info['filename']);
- if (!file_exists($converted_image_uri)) {
+ if (!$this->sourceImageExists($converted_image_uri, $token_is_valid)) {
$this->logger->notice('Source image at %source_image_path not found while trying to generate derivative image at %derivative_path.', ['%source_image_path' => $image_uri, '%derivative_path' => $derivative_uri]);
return new Response($this->t('Error generating image, missing source file.'), 404);
}
@@ -191,9 +208,9 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st
];
// \Drupal\Core\EventSubscriber\FinishResponseSubscriber::onRespond()
// sets response as not cacheable if the Cache-Control header is not
- // already modified. We pass in FALSE for non-private schemes for the
- // $public parameter to make sure we don't change the headers.
- return new BinaryFileResponse($uri, 200, $headers, $scheme !== 'private');
+ // already modified. When $is_public is TRUE, the following sets the
+ // Cache-Control header to "public".
+ return new BinaryFileResponse($uri, 200, $headers, $is_public);
}
else {
$this->logger->notice('Unable to generate the derived image located at %path.', ['%path' => $derivative_uri]);
@@ -201,4 +218,43 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st
}
}
+ /**
+ * Checks whether the provided source image exists.
+ *
+ * @param string $image_uri
+ * The URI for the source image.
+ * @param bool $token_is_valid
+ * Whether a valid image token was supplied.
+ *
+ * @return bool
+ * Whether the source image exists.
+ */
+ private function sourceImageExists(string $image_uri, bool $token_is_valid): bool {
+ $exists = file_exists($image_uri);
+
+ // If the file doesn't exist, we can stop here.
+ if (!$exists) {
+ return FALSE;
+ }
+
+ if ($token_is_valid) {
+ return TRUE;
+ }
+
+ if (StreamWrapperManager::getScheme($image_uri) !== 'public') {
+ return TRUE;
+ }
+
+ $image_path = $this->fileSystem->realpath($image_uri);
+ $private_path = Settings::get('file_private_path');
+ if ($private_path) {
+ $private_path = realpath($private_path);
+ if ($private_path && strpos($image_path, $private_path) === 0) {
+ return FALSE;
+ }
+ }
+
+ return TRUE;
+ }
+
}
diff --git a/core/modules/system/system.module b/core/modules/system/system.module
index f40a4bad4bd4..37a5b5760450 100644
--- a/core/modules/system/system.module
+++ b/core/modules/system/system.module
@@ -27,6 +27,8 @@
use Drupal\Core\Queue\QueueGarbageCollectionInterface;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Routing\StackedRouteMatchInterface;
+use Drupal\Core\Site\Settings;
+use Drupal\Core\StreamWrapper\StreamWrapperManager;
use Drupal\Core\Url;
use GuzzleHttp\Exception\TransferException;
use Symfony\Component\HttpFoundation\RedirectResponse;
@@ -1372,3 +1374,21 @@ function system_page_top() {
}
}
}
+
+/**
+ * Implements hook_file_download().
+ */
+function system_file_download($uri) {
+ $core_schemes = ['public', 'private', 'temporary'];
+ $additional_public_schemes = array_diff(Settings::get('file_additional_public_schemes', []), $core_schemes);
+ if ($additional_public_schemes) {
+ $scheme = StreamWrapperManager::getScheme($uri);
+ if (in_array($scheme, $additional_public_schemes, TRUE)) {
+ return [
+ // Returning any header grants access, and setting the 'Cache-Control'
+ // header is appropriate for public files.
+ 'Cache-Control' => 'public',
+ ];
+ }
+ }
+}
diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php
index 2b610fdf9aeb..447e94eef3b2 100644
--- a/sites/default/default.settings.php
+++ b/sites/default/default.settings.php
@@ -505,6 +505,29 @@
*/
# $settings['file_public_path'] = 'sites/default/files';
+/**
+ * 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.
+ */
+# $settings['file_additional_public_schemes'] = ['example'];
+
/**
* Private file path:
*
--
GitLab