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

Author: pounard

lib/Twig/Loader/Filesystem.php

@@ -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) {
@@ -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);
             }
         }
 
@@ -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)

test/Twig/Tests/Loader/FilesystemTest.php

@@ -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());
+    }
 }