Skip to content

Commit

Permalink
SA-CORE-2022-012 by cmlara, GuyPaddock, larowlan, mondrake, effulgent…
Browse files Browse the repository at this point in the history
…sia, xjm, longwave, Dave Reid, lauriii, David Strauss, benjifisher, alexpott, mcdruid, Fabianx
  • Loading branch information
xjm committed Jul 20, 2022
1 parent 60a00db commit 1f82337
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 13 deletions.
23 changes: 23 additions & 0 deletions core/assets/scaffold/files/default.settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,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:
*
Expand Down
24 changes: 24 additions & 0 deletions core/lib/Drupal/Core/StreamWrapper/PublicStream.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

}
82 changes: 69 additions & 13 deletions core/modules/image/src/Controller/ImageStyleDownloadController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -114,21 +115,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
Expand All @@ -138,26 +143,38 @@ 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();
}
}

// 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);
}
Expand Down Expand Up @@ -196,14 +213,53 @@ 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]);
return new Response($this->t('Error generating image.'), 500);
}
}

/**
* 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;
}

}
20 changes: 20 additions & 0 deletions core/modules/system/system.module
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ use Drupal\Core\PageCache\RequestPolicyInterface;
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;
Expand Down Expand Up @@ -1385,3 +1387,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',
];
}
}
}
23 changes: 23 additions & 0 deletions sites/default/default.settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,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:
*
Expand Down

0 comments on commit 1f82337

Please sign in to comment.