Skip to content

Commit

Permalink
Merge commit from fork
Browse files Browse the repository at this point in the history
Fix GHSA-cw6g-qmjq-6w2w
  • Loading branch information
brandonkelly authored Sep 5, 2024
2 parents dd6c6e2 + ab9d55a commit b5e9a58
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 22 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 2 additions & 15 deletions src/fs/Local.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.'));
}
}

Expand Down
13 changes: 12 additions & 1 deletion src/helpers/Html.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
22 changes: 22 additions & 0 deletions src/services/Security.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
3 changes: 1 addition & 2 deletions src/translations/en/app.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand Down
13 changes: 9 additions & 4 deletions src/web/twig/Extension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 '';
}
}

/**
Expand Down

0 comments on commit b5e9a58

Please sign in to comment.