diff --git a/CHANGELOG.md b/CHANGELOG.md index 30a3be0c32c..e20e0091137 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,11 @@ ## Unreleased +- Added `craft\services\Security::isSystemDir()`. - Fixed a bug where `craft\helpers\StringHelper::lines()` was returning an array of `Stringy\Stringy` objects, rather than strings. - Fixed styling issues with Template field layout UI elements’ selector labels. - Fixed a validation error that could occur when saving a relational field, if the “Maintain hierarchy” setting had been enabled but was no longer applicable. ([#15666](https://github.com/craftcms/cms/issues/15666)) +- Fixed an information disclosure vulnerability. ## 4.12.0 - 2024-09-03 diff --git a/src/fs/Local.php b/src/fs/Local.php index d9185146c8a..35bf9ab8914 100644 --- a/src/fs/Local.php +++ b/src/fs/Local.php @@ -130,21 +130,8 @@ protected function defineRules(): array */ public function validatePath(string $attribute, ?array $params, InlineValidator $validator): void { - // Make sure it’s not within any of the system directories - $path = FileHelper::absolutePath($this->getRootPath(), '/'); - - $systemDirs = Craft::$app->getPath()->getSystemPaths(); - - foreach ($systemDirs as $dir) { - $dir = FileHelper::absolutePath($dir, '/'); - if (str_starts_with("$path/", "$dir/")) { - $validator->addError($this, $attribute, Craft::t('app', 'Local filesystems cannot be located within system directories.')); - break; - } - if (str_starts_with("$dir/", "$path/")) { - $validator->addError($this, $attribute, Craft::t('app', 'Local filesystems cannot be located above system directories.')); - break; - } + if (Craft::$app->getSecurity()->isSystemDir($this->getRootPath())) { + $validator->addError($this, $attribute, Craft::t('app', 'Local filesystems cannot be located within or above system directories.')); } } diff --git a/src/helpers/Html.php b/src/helpers/Html.php index 927e3c6d1d4..10f97ced2ae 100644 --- a/src/helpers/Html.php +++ b/src/helpers/Html.php @@ -990,11 +990,22 @@ public static function dataUrl(string $file, ?string $mimeType = null): string throw new InvalidArgumentException("Invalid file path: $file"); } + $file = FileHelper::absolutePath(Craft::getAlias($file), '/'); + + if (Craft::$app->getSecurity()->isSystemDir(dirname($file))) { + throw new InvalidArgumentException(sprintf('%s cannot be passed a path within or above system directories.', __METHOD__)); + } + + $ext = pathinfo($file, PATHINFO_EXTENSION); + if (strtolower($ext) === 'php') { + throw new InvalidArgumentException(sprintf('%s cannot be passed a path to a PHP file.', __METHOD__)); + } + if ($mimeType === null) { try { $mimeType = FileHelper::getMimeType($file); } catch (Throwable $e) { - Craft::warning("Unable to determine the MIME type for $file: " . $e->getMessage()); + Craft::warning("Unable to determine the MIME type for $file: " . $e->getMessage(), __METHOD__); Craft::$app->getErrorHandler()->logException($e); } } diff --git a/src/services/Security.php b/src/services/Security.php index df065c27f2b..ae3c34691b5 100644 --- a/src/services/Security.php +++ b/src/services/Security.php @@ -8,6 +8,7 @@ namespace craft\services; use Craft; +use craft\helpers\FileHelper; use yii\base\Exception; use yii\base\InvalidArgumentException; use yii\base\InvalidConfigException; @@ -196,4 +197,25 @@ public function redactIfSensitive(string $key, mixed $value): mixed return $value; } + + /** + * Returns whether the given file path is located within or above any system directories. + * + * @param string $path + * @return bool + * @since 4.12.1 + */ + public function isSystemDir(string $path): bool + { + $path = FileHelper::absolutePath($path, '/'); + + foreach (Craft::$app->getPath()->getSystemPaths() as $dir) { + $dir = FileHelper::absolutePath($dir, '/'); + if (str_starts_with("$path/", "$dir/") || str_starts_with("$dir/", "$path/")) { + return true; + } + } + + return false; + } } diff --git a/src/translations/en/app.php b/src/translations/en/app.php index f33f0f15cbe..aa97a41834b 100644 --- a/src/translations/en/app.php +++ b/src/translations/en/app.php @@ -855,8 +855,7 @@ 'Loading' => 'Loading', 'Local Folder' => 'Local Folder', 'Local copies of remote images, generated thumbnails' => 'Local copies of remote images, generated thumbnails', - 'Local filesystems cannot be located above system directories.' => 'Local filesystems cannot be located above system directories.', - 'Local filesystems cannot be located within system directories.' => 'Local filesystems cannot be located within system directories.', + 'Local filesystems cannot be located within or above system directories.' => 'Local filesystems cannot be located within or above system directories.', 'Localizing relations' => 'Localizing relations', 'Location' => 'Location', 'Locations that should be available for previewing entries in this section.' => 'Locations that should be available for previewing entries in this section.', diff --git a/src/web/twig/Extension.php b/src/web/twig/Extension.php index a11f6400d98..72d399cff9d 100644 --- a/src/web/twig/Extension.php +++ b/src/web/twig/Extension.php @@ -1437,11 +1437,16 @@ public function collectFunction(mixed $var): Collection */ public function dataUrlFunction(Asset|string $file, ?string $mimeType = null): string { - if ($file instanceof Asset) { - return $file->getDataUrl(); - } + try { + if ($file instanceof Asset) { + return $file->getDataUrl(); + } - return Html::dataUrl(Craft::getAlias($file), $mimeType); + return Html::dataUrl(Craft::getAlias($file), $mimeType); + } catch (InvalidArgumentException $e) { + Craft::warning($e->getMessage(), __METHOD__); + return ''; + } } /**