sec: many security issues fix

src/Filesystem.php

@@ -36,10 +36,24 @@ public function getFileResolvedPath(string $filename): string|bool
         $paths = (array) $this->config['path'];
 
         foreach ($paths as $path) {
-            $full_filename = realpath($path)  . '/' . $filename . '.' . ltrim($this->getExtension(), '.');
+            $base = realpath($path);
+
+            if ($base === false) {
+                continue;
+            }
+
+            $full_filename = $base . '/' . $filename . '.' . ltrim($this->getExtension(), '.');
             $realpath = realpath($full_filename);
 
-            if ($realpath !== false && file_exists($realpath)) {
+            // Containment check: the resolved file must live inside the
+            // configured view directory. This blocks path traversal from
+            // resolving an arbitrary file, independent of the '.'-to-'/'
+            // rewrite above (which a future change could weaken).
+            if (
+                $realpath !== false
+                && str_starts_with($realpath, $base . DIRECTORY_SEPARATOR)
+                && file_exists($realpath)
+            ) {
                 return $realpath;
             }
         }

src/Lexique/CompileJson.php

@@ -20,7 +20,12 @@ function ($match) {
                 $parts = explode(',', $match[0]);
 
                 if (! isset($parts[1])) {
-                    return  "<?php echo json_encode($parts[0]); ?>";
+                    // Escape HTML-significant characters by default so values
+                    // rendered inside <script> cannot break out of the context
+                    // (XSS). Explicit options passed by the caller are honored.
+                    $flags = 'JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP';
+
+                    return  "<?php echo json_encode($parts[0], $flags); ?>";
                 }
 
                 $options = trim($parts[1]);

src/StackManager.php

@@ -136,10 +136,15 @@ public function endStack()
             $content = $this->pushes[$block];
 
             if (is_null($content)) {
-                $content = $this->tintin->getCompiler()->compile(ob_get_clean());
+                // The block body was compiled and executed together with the
+                // page, so the output buffer already holds its rendered result.
+                // Compiling it a second time would execute any {{ }} that
+                // arrived through data (e() escapes < > & " ' but not braces),
+                // which is remote code execution. Keep the buffer verbatim.
+                $content = ob_get_clean();
             }
 
-            $this->pushes[$block] = trim($content, "\n");
+            $this->pushes[$block] = trim((string) $content, "\n");
         }
     }
 
@@ -152,18 +157,13 @@ public function endStack()
      */
     public function getStack(string $name, ?string $default = null)
     {
-        if (array_key_exists($name, $this->pushes)) {
-            if (is_null($this->pushes[$name])) {
-                $this->pushes[$name] = $default ?? '';
-            }
-
-            return $this->tintin->renderString(
-                $this->pushes[$name],
-                array_merge($this->context, ['__tintin' => $this->tintin])
-            );
+        if (!array_key_exists($name, $this->pushes)) {
+            return $default;
         }
 
-        return $default;
+        // Return the already-rendered block output as-is. Re-rendering it
+        // would execute template syntax that came through data. See endStack().
+        return $this->pushes[$name] ?? ($default ?? '');
     }
 
     /**

src/Tintin.php

@@ -121,50 +121,60 @@ public function getSharedData(): array
      */
     public function render($template, array $data = []): string
     {
-        $__template = $template;
-
         $this->stackManager->setContext($data);
 
         if (is_null($this->loader)) {
             // Try to compile the plain string
-            return $this->renderString($__template, $data);
+            return $this->renderString($template, $data);
         }
 
         // Check existence of file from cache
-        if (! $this->loader->exists($__template)) {
-            $this->loader->failLoading($__template . ' not found');
+        if (! $this->loader->exists($template)) {
+            $this->loader->failLoading($template . ' not found');
         }
 
         // Merge passing data to the shared data
         $this->pushSharedData($data);
 
-        $__tintin = $this;
-
-        // Extract the shared data
-        extract($this->getSharedData());
-
-        // Check if the file cache is not expire
-        // If cache is not still alive we load template
-        // and create the new cache for
-        if (! $this->loader->isExpired($__template)) {
-            $this->obFlushAndStart();
-
-            require $this->loader->getCacheFileResolvedPath($__template);
-
-            return $this->obGetContent();
+        // Resolve and (re)compile from the trusted $template parameter BEFORE
+        // any user data reaches the local scope. Otherwise a data key named
+        // __template would overwrite it through extract() and redirect which
+        // cache file gets required (view-selection hijack).
+        if ($this->loader->isExpired($template)) {
+            $this->loader->cache(
+                $template,
+                $this->compiler->compile($this->loader->getFileContent($template))
+            );
         }
 
-        // Put the template into cache
-        $content = $this->loader->getFileContent($__template);
-
-        $this->loader->cache(
-            $__template,
-            $this->compiler->compile($content)
+        return $this->requireInScope(
+            $this->loader->getCacheFileResolvedPath($template),
+            $this->getSharedData()
         );
+    }
 
+    /**
+     * Require a compiled template in an isolated scope.
+     *
+     * The engine's own variables ($__path, $__tintin, $__data) are defined
+     * before the user data is extracted, and EXTR_SKIP prevents that data from
+     * overwriting them. A data key such as __path or __tintin is therefore
+     * ignored instead of hijacking the include or the engine handle that the
+     * compiled template relies on.
+     *
+     * @param string $__path
+     * @param array $__data
+     * @return string
+     */
+    private function requireInScope(string $__path, array $__data): string
+    {
         $this->obFlushAndStart();
 
-        require $this->loader->getCacheFileResolvedPath($__template);
+        $__tintin = $this;
+
+        extract($__data, EXTR_SKIP);
+
+        require $__path;
 
         return $this->obGetContent();
     }
@@ -191,17 +201,22 @@ public function renderString(string $template, array $data = []): string
      * @param array $data
      * @return string
      */
-    private function executePlainRendering(string $content, array $data): string
+    private function executePlainRendering(string $__content, array $__data): string
     {
         $this->obFlushAndStart();
 
-        extract($data);
+        $__tintin = $this;
+
+        // See requireInScope(): user data must never overwrite the reserved
+        // engine variables, in particular $__content (which becomes the file
+        // that is required) and $__file.
+        extract($__data, EXTR_SKIP);
 
-        $filename = $this->createTmpFile($content);
+        $__file = $this->createTmpFile($__content);
 
-        require $filename;
+        require $__file;
 
-        @unlink($filename);
+        @unlink($__file);
 
         return $this->obGetContent();
     }
@@ -237,10 +252,18 @@ private function createTmpFile(string $content): string
         $tmp_dir = sys_get_temp_dir() . '/__tintin';
 
         if (!is_dir($tmp_dir)) {
-            @mkdir($tmp_dir, 0755, true);
+            @mkdir($tmp_dir, 0700, true);
         }
 
-        $file = $tmp_dir . '/' . md5(microtime(true)) . '.php';
+        // tempnam() atomically creates a unique, unpredictable file owned by
+        // this process (mode 0600). The previous md5(microtime()) name in a
+        // shared, world-readable directory allowed symlink/TOCTOU attacks and
+        // briefly exposed rendered output (possibly secrets) to local users.
+        $file = tempnam($tmp_dir, 'tintin_');
+
+        if ($file === false) {
+            $file = $tmp_dir . '/' . bin2hex(random_bytes(16)) . '.php';
+        }
 
         file_put_contents($file, $content);
 

tests/CompileExtendsTest.php

@@ -111,4 +111,78 @@ public function testShouldRendStack()
         $this->assertEquals('Tintin', $stack_manager->getStack('name'));
         $this->assertEquals('Tintin template', $stack_manager->getStack('pack'));
     }
+
+    /**
+     * A block body is compiled and executed once, together with the page.
+     * Its captured output must be returned as-is and never compiled a second
+     * time, otherwise template syntax that reached the block through data
+     * (e() escapes < > & " ' but not braces) would be executed: RCE.
+     */
+    public function testStackContentIsNotReEvaluated()
+    {
+        $tintin = new Tintin($this->loader);
+
+        $stack_manager = $tintin->getStackManager();
+
+        // This is exactly what the output buffer holds after the first render
+        // when a value like {{ 7*7 }} arrives through the block's data.
+        $stack_manager->startStack('content');
+        echo 'Hello {{ 7*7 }}';
+        $stack_manager->endStack();
+
+        $output = $stack_manager->getStack('content');
+
+        $this->assertStringContainsString('{{ 7*7 }}', $output);
+        $this->assertStringNotContainsString('49', $output);
+    }
+
+    /**
+     * End-to-end: a value carrying template syntax rendered inside a %block
+     * and exposed through %inject must come back as literal text.
+     */
+    public function testBlockDataIsNotExecutedThroughLayout()
+    {
+        $tintin = new Tintin($this->loader);
+
+        $output = $tintin->render('security', ['name' => '{{ 7*7 }}']);
+
+        $this->assertStringContainsString('{{ 7*7 }}', $output);
+        $this->assertStringNotContainsString('49', $output);
+    }
+
+    /**
+     * User data must not be able to overwrite the engine's own $__tintin
+     * handle through extract(); otherwise every block/inject page crashes
+     * (DoS) or the handle can be spoofed.
+     */
+    public function testDataCannotOverwriteEngineHandle()
+    {
+        $tintin = new Tintin($this->loader);
+
+        $output = $tintin->render('security', [
+            'name' => 'Alice',
+            '__tintin' => 'pwned',
+        ]);
+
+        $this->assertStringContainsString('Hello Alice', $output);
+    }
+
+    /**
+     * User data must not be able to redirect which template file is required
+     * by overwriting $__template through extract() (view-selection hijack).
+     */
+    public function testDataCannotHijackTemplateSelection()
+    {
+        $tintin = new Tintin($this->loader);
+
+        $output = $tintin->render('security', [
+            'name' => 'Alice',
+            '__template' => 'layout',
+            '__path' => '/etc/passwd',
+        ]);
+
+        // The 'security' view is rendered, not 'layout' or an injected path.
+        $this->assertStringContainsString('Hello Alice', $output);
+        $this->assertStringNotContainsString('root:', $output);
+    }
 }

tests/CompileJsonTest.php

@@ -22,7 +22,7 @@ public function testCompileJson()
 
         $render = $compile_json->invoke($this->compiler, "%json(['name' => 'tintin'])");
 
-        $this->assertEquals($render, "<?php echo json_encode(['name' => 'tintin']); ?>");
+        $this->assertEquals($render, "<?php echo json_encode(['name' => 'tintin'], JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP); ?>");
     }
 
     public function testCompileJsonWithSpace()
@@ -31,7 +31,7 @@ public function testCompileJsonWithSpace()
 
         $render = $compile_json->invoke($this->compiler, "%json (['name' => 'tintin'])");
 
-        $this->assertEquals($render, "<?php echo json_encode(['name' => 'tintin']); ?>");
+        $this->assertEquals($render, "<?php echo json_encode(['name' => 'tintin'], JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP); ?>");
     }
 
     public function testCompileJsonWithEncodingOptions()

tests/FilesystemTest.php

@@ -74,4 +74,18 @@ public function testGetFileContent()
             'tintin'
         );
     }
+
+    /**
+     * Path resolution must stay inside the configured view directory and
+     * never resolve a file outside it via traversal sequences.
+     */
+    public function testGetFileResolvedPathRejectsTraversal()
+    {
+        $this->assertFalse(
+            $this->filesystem->getFileResolvedPath('../../../../../../etc/passwd')
+        );
+        $this->assertFalse(
+            $this->filesystem->exists('../../../../../../etc/passwd')
+        );
+    }
 }

tests/__snapshots__/CompileMacroTest__testCompileFullImportTemplate__1.txt

@@ -1,10 +1,10 @@
 <!DOCTYPE html>
 <html>
 <head>
-    <title>Extends</title>
+	<title>Extends</title>
 </head>
 <body>
-    Hello, Papac    Sum of 1 + 2 = 3            <div>User's Franck</div>
+	    Hello, Papac    Sum of 1 + 2 = 3            <div>User's Franck</div>
             <div>User's Brice</div>
             <div>User's Lucien</div>
         <input type="text" name="name" value="papac"/></body>

tests/view/security.tintin.php

@@ -0,0 +1,5 @@
+%extends('layout')
+
+%block('content')
+Hello {{ $name }}
+%endblock