diff --git a/app/Services/PhotoPreview.php b/app/Services/PhotoPreview.php index d10424a..57663c4 100644 --- a/app/Services/PhotoPreview.php +++ b/app/Services/PhotoPreview.php @@ -261,19 +261,16 @@ class PhotoPreview private function resolveBinary(string $name): ?string { $candidates = match ($name) { - 'magick' => [base_path('bin/magick'), 'magick', '/opt/homebrew/bin/magick', '/usr/local/bin/magick', '/usr/bin/magick'], - 'convert' => ['convert', '/usr/bin/convert'], - 'heif-convert' => [base_path('bin/heif-convert'), 'heif-convert', '/usr/bin/heif-convert', '/usr/local/bin/heif-convert'], - 'heif-dec' => [base_path('bin/heif-dec'), 'heif-dec', '/usr/bin/heif-dec', '/usr/local/bin/heif-dec'], + 'magick' => [base_path('bin/magick'), '/opt/homebrew/bin/magick', '/usr/local/bin/magick', '/usr/bin/magick', 'magick'], + 'convert' => [base_path('bin/convert'), '/usr/bin/convert', 'convert'], + 'heif-convert' => [base_path('bin/heif-convert'), '/usr/bin/heif-convert', '/usr/local/bin/heif-convert', 'heif-convert'], + 'heif-dec' => [base_path('bin/heif-dec'), '/usr/bin/heif-dec', '/usr/local/bin/heif-dec', 'heif-dec'], default => [$name], }; + $outside = null; $bare = null; foreach ($candidates as $bin) { if (! str_contains($bin, DIRECTORY_SEPARATOR)) { - $found = $this->which($bin); - if ($found !== null) { - return $found; - } $bare ??= $bin; continue; @@ -281,26 +278,13 @@ class PhotoPreview if ($this->isSafeExecutable($bin)) { return $bin; } - } - - // exec() is often allowed when is_executable() is not; let Process try PATH. - return $bare; - } - - private function which(string $name): ?string - { - $path = getenv('PATH'); - if (! is_string($path) || $path === '') { - return null; - } - foreach (explode(PATH_SEPARATOR, $path) as $dir) { - $candidate = rtrim($dir, DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR.$name; - if ($this->isSafeExecutable($candidate)) { - return $candidate; + // Panel open_basedir blocks PHP from stat() /usr/bin, but proc_open can still run it. + if (! $this->isPathInsideOpenBasedir($bin)) { + $outside ??= $bin; } } - return null; + return $outside ?? $bare; } private function isSafeExecutable(string $path): bool @@ -318,14 +302,30 @@ class PhotoPreview if ($basedir === '') { return true; } - $real = realpath($path); - $check = $real !== false ? $real : $path; + $roots = []; foreach (explode(PATH_SEPARATOR, $basedir) as $root) { $root = rtrim($root, DIRECTORY_SEPARATOR); - if ($root === '') { - continue; + if ($root !== '') { + $roots[] = $root; } - if ($check === $root || str_starts_with($check, $root.DIRECTORY_SEPARATOR)) { + } + if ($this->pathPrefixedByRoot($path, $roots)) { + $real = @realpath($path); + $check = is_string($real) && $real !== '' ? $real : $path; + + return $this->pathPrefixedByRoot($check, $roots); + } + + return false; + } + + /** + * @param list $roots + */ + private function pathPrefixedByRoot(string $path, array $roots): bool + { + foreach ($roots as $root) { + if ($path === $root || str_starts_with($path, $root.DIRECTORY_SEPARATOR)) { return true; } } diff --git a/tests/Unit/PhotoPreviewTest.php b/tests/Unit/PhotoPreviewTest.php index 1af13f2..0edd8fc 100644 --- a/tests/Unit/PhotoPreviewTest.php +++ b/tests/Unit/PhotoPreviewTest.php @@ -10,6 +10,18 @@ use Tests\TestCase; class PhotoPreviewTest extends TestCase { + #[Test] + public function basedir_probe_does_not_realpath_system_bins(): void + { + $preview = new PhotoPreview; + $ref = new \ReflectionMethod(PhotoPreview::class, 'isPathInsideOpenBasedir'); + $this->assertFalse($ref->invoke($preview, '/usr/bin/heif-convert') + && (string) ini_get('open_basedir') !== '' + && ! str_contains((string) ini_get('open_basedir'), '/usr/bin')); + $resolved = (new \ReflectionMethod(PhotoPreview::class, 'resolveBinary'))->invoke($preview, 'heif-convert'); + $this->assertNotNull($resolved); + } + #[Test] public function detects_heic_ftyp_header(): void {