From 63e5bcd613f63a1d18527807c6014fcfb0a7818f Mon Sep 17 00:00:00 2001 From: Ambrose Casanova <279373485+ambrose5773@users.noreply.github.com> Date: Sat, 3 Oct 2026 08:55:07 -0600 Subject: [PATCH 1/2] fix: keep view templates inside the views directory Reject absolute template paths and any resolved path outside the configured views directory. --- flight/template/View.php | 74 ++++++++++++++++++++++-- tests/ViewTest.php | 118 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 187 insertions(+), 5 deletions(-) diff --git a/flight/template/View.php b/flight/template/View.php index 17622fd5..24a5724a 100644 --- a/flight/template/View.php +++ b/flight/template/View.php @@ -161,9 +161,14 @@ public function exists(string $file): bool /** * Gets the full path to a template file. * + * Absolute paths are rejected. The resolved file must stay inside the + * configured views directory. If that cannot be shown, this fails closed. + * * @param string $file Template file * * @return string Template file location + * + * @throws \Exception When the path is absolute or resolves outside the views directory. */ public function getTemplate(string $file): string { @@ -173,13 +178,74 @@ public function getTemplate(string $file): string $file .= $ext; } - $is_windows = \strtoupper(\substr(PHP_OS, 0, 3)) === 'WIN'; + if ($this->isAbsolutePath($file)) { + throw new \Exception('Template path is not allowed.'); + } + + $viewsPath = \realpath($this->path); + if ($viewsPath === false || !$this->relativeStaysInside($file)) { + throw new \Exception('Template path is not allowed.'); + } + + $candidate = $this->path . \DIRECTORY_SEPARATOR . $file; + $resolved = \realpath($candidate); + if ($resolved === false) { + return $candidate; + } + + $root = \rtrim($viewsPath, \DIRECTORY_SEPARATOR) . \DIRECTORY_SEPARATOR; + if (\strpos($resolved, $root) !== 0) { + throw new \Exception('Template path is not allowed.'); + } + + return $resolved; + } + + /** + * True when $file is an absolute filesystem path. + */ + private function isAbsolutePath(string $file): bool + { + if ($file === '') { + return false; + } + + if ($file[0] === '/' || $file[0] === '\\') { + return true; + } + + return \strlen($file) > 1 && \ctype_alpha($file[0]) && $file[1] === ':'; + } + + /** + * True when relative segments in $file do not climb out of the views directory. + */ + private function relativeStaysInside(string $file): bool + { + $segments = \preg_split('#[\\/]+#', $file, -1, \PREG_SPLIT_NO_EMPTY); + if ($segments === false) { + return false; + } + + $depth = 0; + foreach ($segments as $segment) { + if ($segment === '.') { + continue; + } + + if ($segment === '..') { + if ($depth === 0) { + return false; + } + + $depth--; + continue; + } - if ((\substr($file, 0, 1) === '/') || ($is_windows && \substr($file, 1, 1) === ':')) { - return $file; + $depth++; } - return $this->path . DIRECTORY_SEPARATOR . $file; + return true; } /** diff --git a/tests/ViewTest.php b/tests/ViewTest.php index 56a6e0e6..60162fb1 100644 --- a/tests/ViewTest.php +++ b/tests/ViewTest.php @@ -111,12 +111,128 @@ public function testTemplateWithCustomExtension(): void $this->expectOutputString("Hello world, Bob!"); } + public function testRenderRelativePathThatStaysInsideViews(): void + { + $this->view->render('layouts/../hello', ['name' => 'Bob']); + + $this->expectOutputString('Hello, Bob!'); + } + public function testGetTemplateAbsolutePath(): void { $tmpfile = tmpfile(); $this->view->extension = ''; $file_path = stream_get_meta_data($tmpfile)['uri']; - $this->assertEquals($file_path, $this->view->getTemplate($file_path)); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Template path is not allowed.'); + $this->view->getTemplate($file_path); + } + + public function testRejectsDriveLetterTemplatePath(): void + { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Template path is not allowed.'); + $this->view->getTemplate('C:' . DIRECTORY_SEPARATOR . 'outside.php'); + } + + public function testRejectsTemplateThatLeavesViewsDirectory(): void + { + $outside = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'flight-view-outside-' . uniqid(); + mkdir($outside); + $note = $outside . DIRECTORY_SEPARATOR . 'note.php'; + file_put_contents($note, 'view->path); + $relative = $this->relativePathFrom($views, $note); + $relative = preg_replace('/\.php$/', '', $relative); + + try { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Template path is not allowed.'); + $this->view->render($relative); + } finally { + unlink($note); + rmdir($outside); + } + } + + public function testRejectsTemplateThatResolvesOutsideViewsDirectory(): void + { + $root = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'flight-view-root-' . uniqid(); + $views = $root . DIRECTORY_SEPARATOR . 'views'; + $outside = $root . DIRECTORY_SEPARATOR . 'outside'; + mkdir($root); + mkdir($views); + mkdir($outside); + $note = $outside . DIRECTORY_SEPARATOR . 'note.php'; + file_put_contents($note, 'removeDir($root); + $this->markTestSkipped('Symlink not available'); + } + + $view = new View($views); + + try { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Template path is not allowed.'); + $view->render('alias'); + } finally { + $this->removeDir($root); + } + } + + public function testRejectsMissingViewsDirectory(): void + { + $view = new View(sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'flight-missing-views-' . uniqid()); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Template path is not allowed.'); + $view->render('hello'); + } + + private function relativePathFrom(string $fromDir, string $toFile): string + { + $from = explode(DIRECTORY_SEPARATOR, rtrim($fromDir, DIRECTORY_SEPARATOR)); + $to = explode(DIRECTORY_SEPARATOR, $toFile); + $file = array_pop($to); + + while ($from !== [] && $to !== [] && $from[0] === $to[0]) { + array_shift($from); + array_shift($to); + } + + $up = array_fill(0, count($from), '..'); + return implode(DIRECTORY_SEPARATOR, array_merge($up, $to, [$file])); + } + + private function removeDir(string $dir): void + { + if (!is_dir($dir)) { + return; + } + + $items = scandir($dir); + if ($items === false) { + return; + } + + foreach ($items as $item) { + if ($item === '.' || $item === '..') { + continue; + } + $path = $dir . DIRECTORY_SEPARATOR . $item; + if (is_link($path) || is_file($path)) { + unlink($path); + continue; + } + $this->removeDir($path); + } + + rmdir($dir); } public function testE(): void From 8093a18fb33206de38055e664ff28be2f3409311 Mon Sep 17 00:00:00 2001 From: Ambrose Casanova <279373485+ambrose5773@users.noreply.github.com> Date: Sat, 3 Oct 2026 08:57:48 -0600 Subject: [PATCH 2/2] fix: reject Windows view paths that leave the views directory Normalize both separators before the containment check, and build the regression next to the views directory so it stays on the same drive. --- flight/template/View.php | 14 ++++++++------ tests/ViewTest.php | 28 +++++++++------------------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/flight/template/View.php b/flight/template/View.php index 24a5724a..2e81ae30 100644 --- a/flight/template/View.php +++ b/flight/template/View.php @@ -222,17 +222,19 @@ private function isAbsolutePath(string $file): bool */ private function relativeStaysInside(string $file): bool { - $segments = \preg_split('#[\\/]+#', $file, -1, \PREG_SPLIT_NO_EMPTY); - if ($segments === false) { - return false; - } - + $segments = \explode('/', \str_replace('\\', '/', $file)); $depth = 0; + foreach ($segments as $segment) { - if ($segment === '.') { + if ($segment === '' || $segment === '.') { continue; } + // A drive letter or colon is an absolute jump, not a view name. + if (\strpos($segment, ':') !== false) { + return false; + } + if ($segment === '..') { if ($depth === 0) { return false; diff --git a/tests/ViewTest.php b/tests/ViewTest.php index 60162fb1..ac360d22 100644 --- a/tests/ViewTest.php +++ b/tests/ViewTest.php @@ -138,14 +138,11 @@ public function testRejectsDriveLetterTemplatePath(): void public function testRejectsTemplateThatLeavesViewsDirectory(): void { - $outside = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'flight-view-outside-' . uniqid(); + $outside = dirname($this->view->path) . DIRECTORY_SEPARATOR . 'flight-view-outside-' . uniqid(); mkdir($outside); $note = $outside . DIRECTORY_SEPARATOR . 'note.php'; file_put_contents($note, 'view->path); - $relative = $this->relativePathFrom($views, $note); - $relative = preg_replace('/\.php$/', '', $relative); + $relative = '..' . DIRECTORY_SEPARATOR . basename($outside) . DIRECTORY_SEPARATOR . 'note'; try { $this->expectException(Exception::class); @@ -157,6 +154,13 @@ public function testRejectsTemplateThatLeavesViewsDirectory(): void } } + public function testRejectsEmbeddedDriveLetter(): void + { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Template path is not allowed.'); + $this->view->getTemplate('layouts' . DIRECTORY_SEPARATOR . 'C:' . DIRECTORY_SEPARATOR . 'outside'); + } + public function testRejectsTemplateThatResolvesOutsideViewsDirectory(): void { $root = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'flight-view-root-' . uniqid(); @@ -194,20 +198,6 @@ public function testRejectsMissingViewsDirectory(): void $view->render('hello'); } - private function relativePathFrom(string $fromDir, string $toFile): string - { - $from = explode(DIRECTORY_SEPARATOR, rtrim($fromDir, DIRECTORY_SEPARATOR)); - $to = explode(DIRECTORY_SEPARATOR, $toFile); - $file = array_pop($to); - - while ($from !== [] && $to !== [] && $from[0] === $to[0]) { - array_shift($from); - array_shift($to); - } - - $up = array_fill(0, count($from), '..'); - return implode(DIRECTORY_SEPARATOR, array_merge($up, $to, [$file])); - } private function removeDir(string $dir): void {