bug #2707 - do not use realpath() per default to avoid conflicts with…

… open_basedir, strong path normalisation
twigphp · Jun 19, 2018 · f16c3e5 · f16c3e5
1 parent 3b71efb
commit f16c3e5
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 9 deletions.
diff --git a/lib/Twig/Loader/Filesystem.php b/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 = $this->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] = $this->normalizePath($filename);
             }
         }
 
@@ -275,6 +283,39 @@ protected function validateName($name)
         }
     }
 
+    /**
+     * Normalize a path by removing redundant '..' and thus preventing the need
+     * of using the realpath() function that may come with some side effects
+     *
+     * @param string $string
+     * @param bool $removeTrailingSlash
+     * @return string
+     */
+    private function normalizePath($string, $removeTrailingSlash = false)
+    {
+        if (DIRECTORY_SEPARATOR !== '/') { // Handle windows gracefully
+            $string = str_replace(DIRECTORY_SEPARATOR, '/', $string);
+        }
+
+        // Preserve scheme
+        // This leaves room for performance improvement
+        $scheme = null;
+        if (strpos($string, '://')) {
+            list($scheme, $string) = explode('://', $string, 2);
+        }
+
+        $count = 0; // This leaves room for performance improvement too
+        do {
+            $string = preg_replace('@[^/]+/+..(/+|$)@', '$2', preg_replace('@//+@', '/', $string), -1, $count);
+        } while ($count);
+
+        if ($removeTrailingSlash) {
+            $string = rtrim($string, '/');
+        }
+
+        return $scheme ? ($scheme.'://'.$string) : $string;
+    }
+
     private function isAbsolutePath($file)
     {
         return strspn($file, '/\\', 0, 1)

diff --git a/test/Twig/Tests/Loader/FilesystemTest.php b/test/Twig/Tests/Loader/FilesystemTest.php
@@ -222,5 +222,19 @@ public function testLoadTemplateFromPhar()
         // $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());
+        // Also attempt filename normalization within a schemed path
+        $this->assertSame('hello from phar', $loader->getSourceContext('hello.twig')->getCode());
+        // And within the filename itself
+    }
+
+    /**
+     * @requires PHP 5.3
+     */
+    public function testLoadTemplateFromPharNormalization()
+    {
+        $loader = new Twig_Loader_Filesystem(array());
+        $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());
     }
 }