From c5e558b903b01eb638fba0b42295efd664fea6fe Mon Sep 17 00:00:00 2001
From: kaibae19 <99116238+kaibae19@users.noreply.github.com>
Date: Thu, 21 May 2026 03:22:47 +0000
Subject: [PATCH 1/2] fix(security): reject archive entries with traversal or
 absolute paths

ZipArchive::extractTo is safe on modern libzip, but the unrar and 7za
codepaths blindly trust archive entry paths and will write outside
$extractTo when an entry begins with "/" or contains a ".." path
component. A user uploading a crafted archive can therefore write into
directories the nc-app process can reach.

Add a per-format pre-extraction listing pass (ZipArchive indices for
ZIP, `unrar lb` for RAR, `7za l -ba -slt` for the rest) and reject the
extraction up front if any entry would escape the destination. The
existing extraction commands and exit-code handling are unchanged.

Signed-off-by: kaibae19 <99116238+kaibae19@users.noreply.github.com>
---
 lib/Service/ExtractionService.php | 103 ++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/lib/Service/ExtractionService.php b/lib/Service/ExtractionService.php
index 276bbd6..fbf76e9 100644
--- a/lib/Service/ExtractionService.php
+++ b/lib/Service/ExtractionService.php
@@ -20,6 +20,90 @@ public function __construct(
 	) {
 	}
 
+	/**
+	 * Reject absolute paths and any ".." path component. Modern libzip
+	 * blocks traversal in ZipArchive::extractTo, but neither unrar nor
+	 * 7za do — a crafted archive can write outside $extractTo.
+	 */
+	private function isUnsafePath(string $entry): bool {
+		$norm = str_replace('\\', '/', $entry);
+		if ($norm === '' || $norm === '.' || $norm === './') {
+			return false;
+		}
+		if (str_starts_with($norm, '/')) {
+			return true;
+		}
+		return (bool)preg_match('#(^|/)\.\.($|/)#', $norm);
+	}
+
+	/** @return null|array{code: 0, desc: string} */
+	private function rejectUnsafe(string $entry): ?array {
+		if (!$this->isUnsafePath($entry)) {
+			return null;
+		}
+		$this->logger->warning('Refusing archive with unsafe entry: ' . $entry);
+		return [
+			'code' => 0,
+			'desc' => $this->l->t('Archive contains an unsafe path and was not extracted'),
+		];
+	}
+
+	/** Validate every ZIP entry via libzip indices. @return null|array{code: 0, desc: string} */
+	private function assertSafeZip(ZipArchive $zip): ?array {
+		for ($i = 0; $i < $zip->numFiles; $i++) {
+			$name = $zip->getNameIndex($i);
+			if ($name === false) {
+				return ['code' => 0, 'desc' => $this->l->t('Failed to read archive index')];
+			}
+			if ($err = $this->rejectUnsafe($name)) {
+				return $err;
+			}
+		}
+		return null;
+	}
+
+	/** Validate every RAR entry via `unrar lb` (bare list). @return null|array{code: 0, desc: string} */
+	private function assertSafeRarShell(string $file): ?array {
+		$output = [];
+		$return = 0;
+		exec('unrar lb ' . escapeshellarg($file) . ' 2>&1', $output, $return);
+		if ($return !== 0) {
+			$this->logger->error('unrar list failed (rc=' . $return . '): ' . implode("\n", $output));
+			return ['code' => 0, 'desc' => $this->l->t('Failed to inspect RAR contents')];
+		}
+		foreach ($output as $line) {
+			$line = trim($line);
+			if ($line === '') {
+				continue;
+			}
+			if ($err = $this->rejectUnsafe($line)) {
+				return $err;
+			}
+		}
+		return null;
+	}
+
+	/** Validate every entry via `7za l -ba -slt`. @return null|array{code: 0, desc: string} */
+	private function assertSafe7z(string $file): ?array {
+		$output = [];
+		$return = 0;
+		exec('7za l -ba -slt ' . escapeshellarg($file) . ' 2>&1', $output, $return);
+		if ($return !== 0) {
+			$this->logger->error('7za list failed (rc=' . $return . '): ' . implode("\n", $output));
+			return ['code' => 0, 'desc' => $this->l->t('Failed to inspect archive contents')];
+		}
+		foreach ($output as $line) {
+			if (!str_starts_with($line, 'Path = ')) {
+				continue;
+			}
+			$entry = substr($line, 7);
+			if ($err = $this->rejectUnsafe($entry)) {
+				return $err;
+			}
+		}
+		return null;
+	}
+
 	/**
 	 * @return (bool|int|mixed)[]
 	 *
@@ -40,6 +124,11 @@ public function extractZip(string $file, string $extractTo): array {
 			return $response;
 		}
 
+		if ($err = $this->assertSafeZip($zip)) {
+			$zip->close();
+			return $err;
+		}
+
 		$success = $zip->extractTo($extractTo);
 		$zip->close();
 		$response = array_merge($response, ['code' => $success ? 1 : 0]);
@@ -55,6 +144,9 @@ public function extractRar(string $file, string $extractTo): array {
 		$response = [];
 
 		if (!extension_loaded('rar')) {
+			if ($err = $this->assertSafeRarShell($file)) {
+				return $err;
+			}
 			exec('unrar x ' . escapeshellarg($file) . ' -R ' . escapeshellarg($extractTo) . '/ -o+', $output, $return);
 			if (sizeof($output) <= 4) {
 				$response = array_merge($response, ['code' => 0, 'desc' => $this->l->t('Oops something went wrong. Check that you have rar extension or unrar installed')]);
@@ -63,6 +155,13 @@ public function extractRar(string $file, string $extractTo): array {
 		} else {
 			$rar_file = rar_open($file);
 			$list = rar_list($rar_file);
+			// Pre-validate every entry before extracting any of them.
+			foreach ($list as $archive_file) {
+				if ($err = $this->rejectUnsafe($archive_file->getName())) {
+					rar_close($rar_file);
+					return $err;
+				}
+			}
 			foreach ($list as $archive_file) {
 				$entry = rar_entry_get($rar_file, $archive_file->getName());
 				$entry->extract($extractTo);
@@ -82,6 +181,10 @@ public function extractRar(string $file, string $extractTo): array {
 	public function extractOther(string $file, string $extractTo): array {
 		$response = [];
 
+		if ($err = $this->assertSafe7z($file)) {
+			return $err;
+		}
+
 		exec('7za -y x ' . escapeshellarg($file) . ' -o' . escapeshellarg($extractTo), $output, $return);
 
 		if (sizeof($output) <= 5) {

From 44d394335aa1c7960d07bd7dbc61d7148db12fa3 Mon Sep 17 00:00:00 2001
From: kaibae19 <99116238+kaibae19@users.noreply.github.com>
Date: Thu, 21 May 2026 03:24:07 +0000
Subject: [PATCH 2/2] fix: use exit codes (not stdout line counts) to detect
 tool failure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

extractRar and extractOther were inferring success from
`sizeof($output) <= 4` / `<= 5` — a brittle heuristic. A successful
extract of a small archive (few stdout lines) can be misreported as a
failure, and a partial/silent failure with verbose stdout can be
misreported as a success.

Capture and check the real exit code from exec(), log the full tool
output on failure, and replace the "Oops" copy with a more actionable
error mentioning the missing tool. Also redirect stderr into $output
so error details from unrar / 7za actually reach the log.

Signed-off-by: kaibae19 <99116238+kaibae19@users.noreply.github.com>
---
 lib/Service/ExtractionService.php | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/lib/Service/ExtractionService.php b/lib/Service/ExtractionService.php
index fbf76e9..930b6e5 100644
--- a/lib/Service/ExtractionService.php
+++ b/lib/Service/ExtractionService.php
@@ -147,9 +147,12 @@ public function extractRar(string $file, string $extractTo): array {
 			if ($err = $this->assertSafeRarShell($file)) {
 				return $err;
 			}
-			exec('unrar x ' . escapeshellarg($file) . ' -R ' . escapeshellarg($extractTo) . '/ -o+', $output, $return);
-			if (sizeof($output) <= 4) {
-				$response = array_merge($response, ['code' => 0, 'desc' => $this->l->t('Oops something went wrong. Check that you have rar extension or unrar installed')]);
+			$output = [];
+			$return = 0;
+			exec('unrar x ' . escapeshellarg($file) . ' -R ' . escapeshellarg($extractTo) . '/ -o+ 2>&1', $output, $return);
+			if ($return !== 0) {
+				$this->logger->error('unrar extract failed (rc=' . $return . '): ' . implode("\n", $output));
+				$response = array_merge($response, ['code' => 0, 'desc' => $this->l->t('Failed to extract RAR archive (is the rar extension or unrar installed?)')]);
 				return $response;
 			}
 		} else {
@@ -185,11 +188,13 @@ public function extractOther(string $file, string $extractTo): array {
 			return $err;
 		}
 
-		exec('7za -y x ' . escapeshellarg($file) . ' -o' . escapeshellarg($extractTo), $output, $return);
+		$output = [];
+		$return = 0;
+		exec('7za -y x ' . escapeshellarg($file) . ' -o' . escapeshellarg($extractTo) . ' 2>&1', $output, $return);
 
-		if (sizeof($output) <= 5) {
-			$response = array_merge($response, ['code' => 0, 'desc' => $this->l->t('Oops something went wrong.')]);
-			$this->logger->error('Is 7-Zip installed? Output: ' . print_r($output, true));
+		if ($return !== 0) {
+			$this->logger->error('7za extract failed (rc=' . $return . '): ' . implode("\n", $output));
+			$response = array_merge($response, ['code' => 0, 'desc' => $this->l->t('Failed to extract archive (is 7-Zip installed?)')]);
 			return $response;
 		}
 		$response = array_merge($response, ['code' => 1]);