Fix: path boundaries

src/php/template.php

@@ -387,24 +387,55 @@ public static function joinPaths(...$params)
   }
 
   /**
-   * Checks if the passed path is above a base directory
+   * Checks if a path is the base directory itself or a descendant.
    * 
-   * $useRealpath resolves the paths using a string-based method
-   * as opposed to calling `realpath()` directly.
+   * Strict checking resolves symlinks; lexical checking only removes dot segments.
    *
    * @param String   $path          The path to check
    * @param String   $base          The base path
    * @param Boolean  $useRealpath   Whether to use realpath
    * 
-   * @return String
+   * @return Boolean
    */ 
   public static function isAboveCurrent($path, $base, $useRealpath = true)
   {
-    return self::startsWith($useRealpath
-      ? realpath($path)
-      : self::removeDotSegments($path), $useRealpath
-        ? realpath($base)
-        : self::removeDotSegments($base));
+    if($path === '' || $base === '')
+    {
+      return false;
+    }
+
+    if($useRealpath)
+    {
+      $path = realpath($path);
+      $base = realpath($base);
+      if($path === false || $base === false)
+      {
+        return false;
+      }
+    }
+
+    /* Windows accepts both separators; on Unix a backslash is a filename character. */
+    if(DIRECTORY_SEPARATOR === '\\')
+    {
+      $path = str_replace('\\', '/', $path);
+      $base = str_replace('\\', '/', $base);
+    }
+
+    if(!$useRealpath)
+    {
+      $path = self::removeDotSegments($path);
+      $base = self::removeDotSegments($base);
+      if($path === '' || $base === '')
+      {
+        return false;
+      }
+    }
+
+    $path = rtrim($path, '/');
+    $base = rtrim($base, '/');
+
+    /* Appending the separator also preserves '/' as a valid base. */
+    return $path === $base || self::startsWith($path, $base . '/');
   }
 
   /**
@@ -908,11 +939,10 @@ function __construct($path, $options = [])
         } else if($options['path_checking'] === 'weak')
         {
           /**
-           * If path checking is 'weak' do another test using a 'realpath' alternative
-           * instead (string-based approach which doesn't solve links)
+           * Weak mode permits external symlink targets only through paths
+           * lexically inside the base directory, without resolving the links.
            */
-          if(self::isAboveCurrent($this->path, $this->relative, false)
-            || is_link($this->path))
+          if(self::isAboveCurrent($this->path, $this->relative, false))
           {
             $this->requested = $requested;
           } else {

tests/php/PathBoundariesTest.php

@@ -0,0 +1,77 @@
+<?php
+declare(strict_types=1);
+
+use PHPUnit\Framework\TestCase;
+
+final class PathBoundariesTest extends TestCase
+{
+    public function testBoundaryComparisonInStrictAndWeakModes(): void
+    {
+        $root = sys_get_temp_dir() . '/ivfi-boundaries-' . bin2hex(random_bytes(8));
+        $directories = [$root, $root . '/base', $root . '/base/child', $root . '/base-other'];
+        foreach($directories as $directory) {
+            mkdir($directory, 0700);
+        }
+
+        try {
+            $base = $root . '/base';
+            $cases = [];
+            $expected = [];
+            foreach(['strict' => true, 'weak' => false] as $mode => $resolve) {
+                foreach([
+                    'base itself' => [$base, $base, true],
+                    'descendant' => [$base . '/child', $base, true],
+                    'trailing separator' => [$base, $base . '/', true],
+                    'trailing separator on target' => [$base . '/child/', $base, true],
+                    'sibling with shared prefix' => [$root . '/base-other', $base, false],
+                    'traversal to sibling' => [$base . '/../base-other', $base, false],
+                    'parent' => [$root, $base, false],
+                    'empty base' => [$base, '', false],
+                    'empty target' => ['', $base, false],
+                ] as $name => [$path, $boundary, $allowed]) {
+                    $cases[$mode . ': ' . $name] = [$path, $boundary, $resolve];
+                    $expected[$mode . ': ' . $name] = $allowed;
+                }
+            }
+            foreach(['missing target' => [$root . '/missing', $base, true],
+                'missing base' => [$base, $root . '/missing', true]] as $name => $case) {
+                $cases[$name] = $case;
+                $expected[$name] = false;
+            }
+            if(DIRECTORY_SEPARATOR === '/') {
+                foreach([true, false] as $resolve) {
+                    $name = 'filesystem root ' . (int) $resolve;
+                    $cases[$name] = [$base, '/', $resolve];
+                    $expected[$name] = true;
+                    $cases[$name . ' itself'] = ['/', '/', $resolve];
+                    $expected[$name . ' itself'] = true;
+                }
+            }
+
+            $build = dirname(__DIR__, 2) . '/build/indexer.php';
+            self::assertFileExists($build, 'Run pnpm build before the PHP tests.');
+            $process = proc_open(
+                [PHP_BINARY, __DIR__ . '/check-boundaries.php', $build],
+                [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']],
+                $pipes, $root
+            );
+            self::assertIsResource($process);
+            fwrite($pipes[0], json_encode(['uri' => '/', 'prepend' => '', 'cases' => $cases], JSON_THROW_ON_ERROR));
+            fclose($pipes[0]);
+            $output = stream_get_contents($pipes[1]);
+            $errors = stream_get_contents($pipes[2]);
+            fclose($pipes[1]);
+            fclose($pipes[2]);
+            self::assertSame(0, proc_close($process), $errors);
+            self::assertSame('', $errors);
+            $results = json_decode($output, true, 512, JSON_THROW_ON_ERROR);
+            foreach($expected as $name => $allowed) {
+                self::assertSame($allowed, $results[$name], $name);
+            }
+        } finally {
+            foreach(array_reverse($directories) as $directory) {
+                rmdir($directory);
+            }
+        }
+    }
+}

tests/php/SymlinkPathsTest.php

@@ -0,0 +1,85 @@
+<?php
+declare(strict_types=1);
+
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+final class SymlinkPathsTest extends TestCase
+{
+    public static function paths(): array
+    {
+        return [
+            'strict internal link' => ['strict', '/internal-link/', 200],
+            'weak internal link' => ['weak', '/internal-link/', 200],
+            'strict external link' => ['strict', '/external-link/', 403],
+            'weak external link' => ['weak', '/external-link/', 200],
+            'strict external descendant' => ['strict', '/external-link/child/', 403],
+            'weak external descendant' => ['weak', '/external-link/child/', 200],
+            'strict outside link' => ['strict', '/../outside-link/', 403],
+            'weak outside link' => ['weak', '/../outside-link/', 403],
+        ];
+    }
+
+    #[DataProvider('paths')]
+    public function testDirectorySymlinkPolicy(string $mode, string $uri, int $status): void
+    {
+        $root = sys_get_temp_dir() . '/ivfi-symlinks-' . bin2hex(random_bytes(8));
+        $created = [];
+        try {
+            foreach([$root, "$root/base", "$root/base/internal", "$root/external", "$root/external/child"] as $directory) {
+                mkdir($directory, 0700);
+                $created[] = $directory;
+            }
+            foreach([
+                "$root/base/internal-link" => "$root/base/internal",
+                "$root/base/external-link" => "$root/external",
+                "$root/outside-link" => "$root/external",
+            ] as $link => $target) {
+                self::assertTrue(symlink($target, $link));
+                $created[] = $link;
+            }
+            foreach(["$root/base/internal", "$root/external", "$root/external/child"] as $directory) {
+                file_put_contents("$directory/visible-marker.txt", 'fixture');
+                $created[] = "$directory/visible-marker.txt";
+            }
+            $config = "$root/base/.indexer.config.php";
+            file_put_contents($config, '<?php return ' . var_export(['path_checking' => $mode], true) . ';');
+            $created[] = $config;
+
+            $build = dirname(__DIR__, 2) . '/build/indexer.php';
+            self::assertFileExists($build, 'Run pnpm build before the PHP tests.');
+            $process = proc_open(
+                [PHP_BINARY, __DIR__ . '/render.php', $build],
+                [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']],
+                $pipes, "$root/base"
+            );
+            self::assertIsResource($process);
+            fwrite($pipes[0], json_encode(['uri' => $uri, 'prepend' => '', 'captureResponse' => true], JSON_THROW_ON_ERROR));
+            fclose($pipes[0]);
+            $output = stream_get_contents($pipes[1]);
+            $errors = stream_get_contents($pipes[2]);
+            fclose($pipes[1]);
+            fclose($pipes[2]);
+            self::assertSame(0, proc_close($process), $errors);
+            self::assertSame('', $errors);
+            $response = json_decode($output, true, 512, JSON_THROW_ON_ERROR);
+            self::assertSame($status, $response['status']);
+            if($status === 200) {
+                // Check both the listing and its public link, not just the status.
+                self::assertStringContainsString('href="' . $uri . 'visible-marker.txt"', $response['body']);
+                self::assertStringContainsString('data-raw="visible-marker.txt"', $response['body']);
+            } else {
+                self::assertSame('<h3>403 Forbidden</h3>', $response['body']);
+            }
+        } finally {
+            // Unlink fixtures explicitly; never follow a symlink during cleanup.
+            foreach(array_reverse($created) as $path) {
+                if(is_link($path) || is_file($path)) {
+                    unlink($path);
+                } else {
+                    rmdir($path);
+                }
+            }
+        }
+    }
+}

tests/php/check-boundaries.php

@@ -0,0 +1,11 @@
+<?php
+// Load the real generated page, discard its HTML, then exercise its helper.
+ob_start();
+require __DIR__ . '/render.php';
+ob_end_clean();
+
+$results = [];
+foreach($request['cases'] as $name => $case) {
+    $results[$name] = Helpers::isAboveCurrent($case[0], $case[1], $case[2]);
+}
+echo json_encode($results, JSON_THROW_ON_ERROR);