Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug #2707 - do not use realpath() per default to avoid conflicts with open_basedir, strong path normalisation #2709

Open
wants to merge 1 commit into
base: 1.x
Choose a base branch
from
Open

bug #2707 - do not use realpath() per default to avoid conflicts with open_basedir, strong path normalisation #2709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 59 additions & 9 deletions lib/Twig/Loader/Filesystem.php
View file
Open in desktop
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
View file
Open in desktop
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());
}
}