Skip to content

Commit

Permalink
bug #2707 - do not use realpath() per default to avoid conflicts with…
Browse files Browse the repository at this point in the history
… open_basedir, strong path normalisation
  • Loading branch information
pounard committed Jun 26, 2018
1 parent 3b71efb commit 5973ad3
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 11 deletions.
68 changes: 59 additions & 9 deletions lib/Twig/Loader/Filesystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,25 @@ class Twig_Loader_Filesystem implements Twig_LoaderInterface, Twig_ExistsLoaderI
protected $cache = array();
protected $errorCache = array();

private $useRealpath = false;
private $rootPath;

/**
* @param string|array $paths A path or an array of paths where to look for templates
* @param string|null $rootPath The root path common to all relative paths (null for getcwd())
*/
public function __construct($paths = array(), $rootPath = null)
public function __construct($paths = array(), $rootPath = null, $useRealpath = false)
{
$this->rootPath = (null === $rootPath ? getcwd() : $rootPath).DIRECTORY_SEPARATOR;
if (false !== $realPath = realpath($rootPath)) {
$this->rootPath = $realPath.DIRECTORY_SEPARATOR;
$this->useRealpath = $useRealpath;

$this->rootPath = (null === $rootPath ? getcwd() : $rootPath);
if ($this->rootPath) {
// realpath() usage for backward compatibility only
if ($useRealpath && false !== ($realPath = realpath($this->rootPath))) {
$this->rootPath = $realPath.DIRECTORY_SEPARATOR;
} else {
$this->rootPath = self::normalizePath($this->rootPath).DIRECTORY_SEPARATOR;
}
}

if ($paths) {
Expand Down Expand Up @@ -214,12 +222,12 @@ protected function findTemplate($name)
$path = $this->rootPath.'/'.$path;
}

if (is_file($path.'/'.$shortname)) {
if (false !== $realpath = realpath($path.'/'.$shortname)) {
return $this->cache[$name] = $realpath;
$filename = $path.'/'.$shortname;
if (is_file($filename)) {
if ($this->useRealpath && false !== ($realPath = realpath($filename))) {
$filename = $realPath;
}

return $this->cache[$name] = $path.'/'.$shortname;
return $this->cache[$name] = self::normalizePath($filename);
}
}

Expand Down Expand Up @@ -275,6 +283,48 @@ protected function validateName($name)
}
}

/**
* Normalize a path by removing redundant '..', '.' and '/' and thus preventing the
* need of using the realpath() function that may come with some side effects such
* as breaking out open_basedir configuration by attempting to following symlinks
*
* @param string $string
* @param bool $removeTrailingSlash
* @return string
*/
static public function normalizePath($string)
{
// Handle windows gracefully
if (\DIRECTORY_SEPARATOR !== '/') {
$string = \str_replace(\DIRECTORY_SEPARATOR, '/', $string);
}
// Also tests some special cases we can't really do anything with
if (false === \strpos($string, '/') || '/' === $string || '.' === $string || '..' === $string) {
return $string;
}
// This is supposedly invalid, but an empty string is an empty string
if ('' === ($string = \rtrim($string, '/'))) {
return '';
}

$scheme = null;
if (\strpos($string, '://')) {
list($scheme, $string) = \explode('://', $string, 2);
}

// Matches useless '.' repetitions
$string = \preg_replace('@^\./|(/\.)+/|/\.$@', '/', $string);

$count = 0;
do {
// string such as '//' can be generated by the first regex, hence the second
$string = \preg_replace('@[^/]+/+\.\.(/+|$)@', '$2', \preg_replace('@//+@', '/', $string), -1, $count);
} while ($count);

// rtrim() a second time because preg_replace() could leave a trailing '/'
return ($scheme ? ($scheme.'://') : '').\rtrim($string, '/');
}

private function isAbsolutePath($file)
{
return strspn($file, '/\\', 0, 1)
Expand Down
61 changes: 59 additions & 2 deletions test/Twig/Tests/Loader/FilesystemTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,16 +211,73 @@ public function testArrayInheritance($templateName)
$this->assertSame('VALID Child', $template->renderBlock('body', array()));
}

public function getPathNormalizationMap()
{
return array(
// Tests with '..'
array('a/b/..', 'a'),
array('https://a/b/../', 'https://a'),
array('/a/b/c/d/../e/f', '/a/b/c/e/f'),
array('a/b/c/../../e/f', 'a/e/f'),
array('ftp://a/../b/../c/../e/f', 'ftp://e/f'),
array('a../b/c../d..e/', 'a../b/c../d..e'),
array('../c/d', '../c/d'),
// With multiple '/'
array('/a/b/////c/d/../e/f', '/a/b/c/e/f'),
array('file:////a/b/c//../..//e/f', 'file:///a/e/f'),
array('////a/../b/../c//../e/f', '/e/f'),
array('a../b//c../d..e/', 'a../b/c../d..e'),
array('../c////d', '../c/d'),
// With dots
array('a/b/./././..', 'a'),
array('a/.b/./../', 'a'),
array('/a/b/.c/d/../e/f', '/a/b/.c/e/f'),
array('.a/./b/c/.././../e./f', '.a/e./f'),
// Special cases
array('/', '/'),
array('.', '.'),
array('..', '..'),
array('./', '.'),
array('../', '..'),
);
}

/**
* @dataProvider getPathNormalizationMap
*/
public function testNormalizePath($path, $expected)
{
$this->assertSame($expected, Twig_Loader_Filesystem::normalizePath($path));
}

public function getUseRealpathConfiguration()
{
return array(array(true), array(false));
}

/**
* @dataProvider getUseRealpathConfiguration
* @requires PHP 5.3
*/
public function testLoadTemplateFromPhar()
public function testLoadTemplateFromPhar($useRealpath)
{
$loader = new Twig_Loader_Filesystem(array());
$loader = new Twig_Loader_Filesystem(array(), null, $useRealpath);
// phar-sample.phar was created with the following script:
// $f = new Phar('phar-test.phar');
// $f->addFromString('hello.twig', 'hello from phar');
$loader->addPath('phar://'.dirname(__FILE__).'/Fixtures/phar/phar-sample.phar');
$this->assertSame('hello from phar', $loader->getSourceContext('hello.twig')->getCode());
}

/**
* @dataProvider getUseRealpathConfiguration
* @requires PHP 5.3
*/
public function testLoadTemplateFromPharNormalization($useRealpath)
{
$loader = new Twig_Loader_Filesystem(array(), null, $useRealpath);
$loader->addPath('phar://'.dirname(__FILE__).'/Fixtures/phar/non-existing-segment/../phar-sample.phar');
$this->assertSame('hello from phar', $loader->getSourceContext('hello.twig')->getCode());
$this->assertSame('hello from phar', $loader->getSourceContext('another-non-existing-segment/../hello.twig')->getCode());
}
}

0 comments on commit 5973ad3

Please sign in to comment.