From 7fdaf626843efbbd3717625161bd0246420cd97a Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Mon, 12 Apr 2021 16:07:36 +0300 Subject: [PATCH] Restrict filesystem Twig functions to accept only local filesystem and grav streams --- CHANGELOG.md | 1 + .../Twig/Extension/FilesystemExtension.php | 351 ++++++++++++++++++ .../GravExtension.php} | 6 +- system/src/Grav/Common/Twig/Twig.php | 5 +- system/src/Grav/Common/Utils.php | 81 ++-- 5 files changed, 403 insertions(+), 41 deletions(-) create mode 100644 system/src/Grav/Common/Twig/Extension/FilesystemExtension.php rename system/src/Grav/Common/Twig/{TwigExtension.php => Extension/GravExtension.php} (99%) diff --git a/CHANGELOG.md b/CHANGELOG.md index adda45d6f..ef5ec8a3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Added configuration options to allow PHP methods to be used in Twig functions (`system.twig.safe_functions`) and filters (`system.twig.safe_filters`) * Deprecated using PHP methods in Twig without them being in the safe lists * Prevent dangerous PHP methods from being used as Twig functions and filters + * Restrict filesystem Twig functions to accept only local filesystem and grav streams 1. [](#improved) * Better GPM detection of unauthorized installations 1. [](#bugfix) diff --git a/system/src/Grav/Common/Twig/Extension/FilesystemExtension.php b/system/src/Grav/Common/Twig/Extension/FilesystemExtension.php new file mode 100644 index 000000000..bdebae217 --- /dev/null +++ b/system/src/Grav/Common/Twig/Extension/FilesystemExtension.php @@ -0,0 +1,351 @@ +locator = Grav::instance()['locator']; + } + + /** + * @return TwigFunction[] + */ + public function getFilters() + { + return $this->getFunctions(); + } + + /** + * Return a list of all functions. + * + * @return TwigFunction[] + */ + public function getFunctions() + { + return [ + new TwigFunction('file_exists', [$this, 'file_exists']), + new TwigFunction('fileatime', [$this, 'fileatime']), + new TwigFunction('filectime', [$this, 'filectime']), + new TwigFunction('filemtime', [$this, 'filemtime']), + new TwigFunction('filesize', [$this, 'filesize']), + new TwigFunction('filetype', [$this, 'filetype']), + new TwigFunction('is_dir', [$this, 'is_dir']), + new TwigFunction('is_file', [$this, 'is_file']), + new TwigFunction('is_link', [$this, 'is_link']), + new TwigFunction('is_readable', [$this, 'is_readable']), + new TwigFunction('is_writable', [$this, 'is_writable']), + new TwigFunction('is_writeable', [$this, 'is_writable']), + new TwigFunction('lstat', [$this, 'lstat']), + new TwigFunction('getimagesize', [$this, 'getimagesize']), + new TwigFunction('exif_read_data', [$this, 'exif_read_data']), + new TwigFunction('read_exif_data', [$this, 'exif_read_data']), + new TwigFunction('exif_imagetype', [$this, 'exif_imagetype']), + new TwigFunction('hash_file', [$this, 'hash_file']), + new TwigFunction('hash_hmac_file', [$this, 'hash_hmac_file']), + new TwigFunction('md5_file', [$this, 'md5_file']), + new TwigFunction('sha1_file', [$this, 'sha1_file']), + new TwigFunction('get_meta_tags', [$this, 'get_meta_tags']), + ]; + } + + /** + * @param string $filename + * @return bool + */ + public function file_exists($filename): bool + { + if (!$this->checkFilename($filename)) { + return false; + } + + return file_exists($filename); + } + + /** + * @param string $filename + * @return int|false + */ + public function fileatime($filename) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return fileatime($filename); + } + + /** + * @param string $filename + * @return int|false + */ + public function filectime($filename) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return filectime($filename); + } + + /** + * @param string $filename + * @return int|false + */ + public function filemtime($filename) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return filemtime($filename); + } + + /** + * @param string $filename + * @return int|false + */ + public function filesize($filename) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return filesize($filename); + } + + /** + * @param string $filename + * @return string|false + */ + public function filetype($filename) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return filetype($filename); + } + + /** + * @param string $filename + * @return bool + */ + public function is_dir($filename): bool + { + if (!$this->checkFilename($filename)) { + return false; + } + + return is_dir($filename); + } + + /** + * @param string $filename + * @return bool + */ + public function is_file($filename): bool + { + if (!$this->checkFilename($filename)) { + return false; + } + + return is_file($filename); + } + + /** + * @param string $filename + * @return bool + */ + public function is_link($filename): bool + { + if (!$this->checkFilename($filename)) { + return false; + } + + return is_link($filename); + } + + /** + * @param string $filename + * @return bool + */ + public function is_readable($filename): bool + { + if (!$this->checkFilename($filename)) { + return false; + } + + return is_readable($filename); + } + + /** + * @param string $filename + * @return bool + */ + public function is_writable($filename): bool + { + if (!$this->checkFilename($filename)) { + return false; + } + + return is_writable($filename); + } + + /** + * @param string $filename + * @return array|false + */ + public function lstat($filename) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return lstat($filename); + } + + /** + * @param string $filename + * @return array|false + */ + public function getimagesize($filename) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return getimagesize($filename); + } + + /** + * @param string $file + * @param string|null $required_sections + * @param bool $as_arrays + * @param bool $read_thumbnail + * @return array|false + */ + public function exif_read_data($file, ?string $required_sections, bool $as_arrays = false, bool $read_thumbnail = false) + { + if (!Utils::functionExists('exif_read_data') || !$this->checkFilename($file)) { + return false; + } + + return exif_read_data($file, $required_sections, $as_arrays, $read_thumbnail); + } + + /** + * @param string $filename + * @return string|false + */ + public function exif_imagetype($filename) + { + if (!Utils::functionExists('exif_imagetype') || !$this->checkFilename($filename)) { + return false; + } + + return @exif_imagetype(); + } + + /** + * @param string $algo + * @param string $filename + * @param bool $binary + * @return string|false + */ + public function hash_file(string $algo, string $filename, bool $binary = false) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return hash_file($algo, $filename, $binary); + } + + /** + * @param string $algo + * @param string $data + * @param string $key + * @param bool $binary + * @return string|false + */ + public function hash_hmac_file(string $algo, string $data, string $key, bool $binary = false) + { + if (!$this->checkFilename($data)) { + return false; + } + + return hash_hmac_file($algo, $data, $key, $binary); + } + + /** + * @param string $filename + * @param bool $binary + * @return string|false + */ + public function md5_file($filename, bool $binary = false) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return md5_file($filename, $binary); + } + + /** + * @param string $filename + * @param bool $binary + * @return string|false + */ + public function sha1_file($filename, bool $binary = false) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return sha1_file($filename, $binary); + } + + /** + * @param string $filename + * @return array|false + */ + public function get_meta_tags($filename) + { + if (!$this->checkFilename($filename)) { + return false; + } + + return get_meta_tags($filename); + } + + /** + * @param string $filename + * @return bool + */ + private function checkFilename($filename): bool + { + return is_string($filename) && (!str_contains($filename, '://') || $this->locator->isStream($filename)); + } +} diff --git a/system/src/Grav/Common/Twig/TwigExtension.php b/system/src/Grav/Common/Twig/Extension/GravExtension.php similarity index 99% rename from system/src/Grav/Common/Twig/TwigExtension.php rename to system/src/Grav/Common/Twig/Extension/GravExtension.php index 4c6fa9e00..febdeded7 100644 --- a/system/src/Grav/Common/Twig/TwigExtension.php +++ b/system/src/Grav/Common/Twig/Extension/GravExtension.php @@ -7,7 +7,7 @@ * @license MIT License; see LICENSE file for details. */ -namespace Grav\Common\Twig; +namespace Grav\Common\Twig\Extension; use Cron\CronExpression; use Grav\Common\Config\Config; @@ -60,14 +60,13 @@ use function is_numeric; use function is_object; use function is_scalar; use function is_string; -use function ord; use function strlen; /** * Class TwigExtension * @package Grav\Common\Twig */ -class TwigExtension extends AbstractExtension implements GlobalsInterface +class GravExtension extends AbstractExtension implements GlobalsInterface { /** @var Grav */ protected $grav; @@ -214,7 +213,6 @@ class TwigExtension extends AbstractExtension implements GlobalsInterface new TwigFunction('svg_image', [$this, 'svgImageFunction']), new TwigFunction('xss', [$this, 'xssFunc']), - // Translations new TwigFunction('t', [$this, 'translate'], ['needs_environment' => true]), new TwigFunction('tl', [$this, 'translateLanguage']), diff --git a/system/src/Grav/Common/Twig/Twig.php b/system/src/Grav/Common/Twig/Twig.php index a99068725..8cbd8d662 100644 --- a/system/src/Grav/Common/Twig/Twig.php +++ b/system/src/Grav/Common/Twig/Twig.php @@ -16,6 +16,8 @@ use Grav\Common\Language\Language; use Grav\Common\Language\LanguageCodes; use Grav\Common\Page\Interfaces\PageInterface; use Grav\Common\Page\Pages; +use Grav\Common\Twig\Extension\FilesystemExtension; +use Grav\Common\Twig\Extension\GravExtension; use Grav\Common\Utils; use RocketTheme\Toolbox\ResourceLocator\UniformResourceLocator; use RocketTheme\Toolbox\Event\Event; @@ -217,7 +219,8 @@ class Twig if ($config->get('system.twig.debug')) { $this->twig->addExtension(new DebugExtension()); } - $this->twig->addExtension(new TwigExtension()); + $this->twig->addExtension(new GravExtension()); + $this->twig->addExtension(new FilesystemExtension()); $this->twig->addExtension(new DeferredExtension()); $this->twig->addExtension(new StringLoaderExtension()); diff --git a/system/src/Grav/Common/Utils.php b/system/src/Grav/Common/Utils.php index 383107cfd..edc093e4f 100644 --- a/system/src/Grav/Common/Utils.php +++ b/system/src/Grav/Common/Utils.php @@ -1963,6 +1963,35 @@ abstract class Utils 'posix_setuid', ]; + if (in_array($name, $commandExecutionFunctions)) { + return true; + } + + if (in_array($name, $codeExecutionFunctions)) { + return true; + } + + if (isset($callbackFunctions[$name])) { + return true; + } + + if (in_array($name, $informationDiscosureFunctions)) { + return true; + } + + if (in_array($name, $otherFunctions)) { + return true; + } + + return static::isFilesystemFunction($name); + } + + /** + * @param string $name + * @return bool + */ + public static function isFilesystemFunction(string $name): bool + { static $fileWriteFunctions = [ 'fopen', 'tmpfile', @@ -2031,53 +2060,33 @@ abstract class Utils static $filesystemFunctions = [ // read from filesystem - //'file_exists', - //'fileatime', - //'filectime', - //'filemtime', - //'filesize', - //'filetype', - //'is_dir', - //'is_file', + 'file_exists', + 'fileatime', + 'filectime', + 'filemtime', + 'filesize', + 'filetype', + 'is_dir', + 'is_file', 'is_link', - //'is_readable', + 'is_readable', 'is_writable', 'is_writeable', 'linkinfo', 'lstat', //'pathinfo', - //'getimagesize', - //'exif_read_data', - //'read_exif_data', + 'getimagesize', + 'exif_read_data', + 'read_exif_data', 'exif_thumbnail', 'exif_imagetype', - //'hash_file', - //'hash_hmac_file', - //'md5_file', - //'sha1_file', + 'hash_file', + 'hash_hmac_file', + 'md5_file', + 'sha1_file', 'get_meta_tags', ]; - if (in_array($name, $commandExecutionFunctions)) { - return true; - } - - if (in_array($name, $codeExecutionFunctions)) { - return true; - } - - if (isset($callbackFunctions[$name])) { - return true; - } - - if (in_array($name, $informationDiscosureFunctions)) { - return true; - } - - if (in_array($name, $otherFunctions)) { - return true; - } - if (in_array($name, $fileWriteFunctions)) { return true; }