diff --git a/src/php/template.php b/src/php/template.php index 525ff52..f018d1a 100755 --- a/src/php/template.php +++ b/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 { diff --git a/tests/php/PathBoundariesTest.php b/tests/php/PathBoundariesTest.php new file mode 100644 index 0000000..e4146a7 --- /dev/null +++ b/tests/php/PathBoundariesTest.php @@ -0,0 +1,77 @@ + 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); + } + } + } +} diff --git a/tests/php/SymlinkPathsTest.php b/tests/php/SymlinkPathsTest.php new file mode 100644 index 0000000..1fea36b --- /dev/null +++ b/tests/php/SymlinkPathsTest.php @@ -0,0 +1,85 @@ + ['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, ' $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('

403 Forbidden

', $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); + } + } + } + } +} diff --git a/tests/php/check-boundaries.php b/tests/php/check-boundaries.php new file mode 100644 index 0000000..e9c7f99 --- /dev/null +++ b/tests/php/check-boundaries.php @@ -0,0 +1,11 @@ + $case) { + $results[$name] = Helpers::isAboveCurrent($case[0], $case[1], $case[2]); +} +echo json_encode($results, JSON_THROW_ON_ERROR);