fix(security): refuse top-level asset loads and cap screenshot reads

lib/Controller/AssetController.php

@@ -69,6 +69,13 @@ public function fetch(string $sessionId, string $path): DataDisplayResponse|Data
 		if ($user === null) {
 			return new DataResponse(['error' => 'Not authenticated'], Http::STATUS_UNAUTHORIZED);
 		}
+		// Package HTML may only render inside the viewer iframe. Opened as a
+		// top-level page it would run author scripts on the Nextcloud origin.
+		// ponytail: browsers without Fetch Metadata pass; the opaque-origin
+		// sandbox is the real fix.
+		if ($this->request->getHeader('Sec-Fetch-Dest') === 'document') {
+			return new DataResponse(['error' => 'Not embeddable here'], Http::STATUS_FORBIDDEN);
+		}
 		$fileId = (int)$sessionId;
 		if ($fileId <= 0) {
 			return new DataResponse(['error' => 'Invalid session'], Http::STATUS_BAD_REQUEST);

lib/Controller/ThumbnailController.php

@@ -16,6 +16,7 @@
 use OCP\Files\NotPermittedException;
 use OCP\IRequest;
 use OCP\IUserSession;
+use RuntimeException;
 
 /**
  * Returns the `screenshot.png` from inside an `.elpx` package, suitable for
@@ -49,7 +50,11 @@ public function byFileId(int $fileId): DataDisplayResponse|DataResponse {
 			return new DataResponse(['error' => $e->getMessage()], Http::STATUS_FORBIDDEN);
 		}
 
-		$bytes = $this->zipEntries->readEntry($file, 'screenshot.png');
+		try {
+			$bytes = $this->zipEntries->readEntry($file, 'screenshot.png', ZipEntryService::MAX_SCREENSHOT_BYTES);
+		} catch (RuntimeException) {
+			$bytes = null;
+		}
 		if ($bytes === null) {
 			return new DataResponse(['error' => 'No screenshot'], Http::STATUS_NOT_FOUND);
 		}

lib/Preview/ElpxPreviewProvider.php

@@ -59,7 +59,7 @@ public function isAvailable(\OCP\Files\FileInfo $file): bool {
 
 	public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		try {
-			$bytes = $this->zipEntries->readEntry($file, 'screenshot.png');
+			$bytes = $this->zipEntries->readEntry($file, 'screenshot.png', ZipEntryService::MAX_SCREENSHOT_BYTES);
 			if ($bytes !== null) {
 				$image = new Image();
 				$image->loadFromData($bytes);

lib/Service/ZipEntryService.php

@@ -18,6 +18,12 @@
 class ZipEntryService {
 	public const MAX_ENTRIES = 5000;
 	public const MAX_UNCOMPRESSED_SIZE_BYTES = 500 * 1024 * 1024;
+	/**
+	 * Cap for `screenshot.png`. Thumbnails run in cron and the Files grid, so
+	 * a package declaring a huge screenshot must not reserve a buffer anywhere
+	 * near `memory_limit`.
+	 */
+	public const MAX_SCREENSHOT_BYTES = 10 * 1024 * 1024;
 
 	/**
 	 * The limits are injectable so tests can exercise them with small
@@ -36,16 +42,18 @@ public function __construct(
 	 * Entry names that are not already canonical — traversal, absolute paths,
 	 * dot segments, doubled slashes, backslashes — are rejected outright by
 	 * {@see self::normalizeEntry()} rather than repaired.
+	 *
+	 * `$maxBytes` tightens the uncompressed-size limit for this one read.
 	 */
-	public function readEntry(File $file, string $entry): ?string {
+	public function readEntry(File $file, string $entry, ?int $maxBytes = null): ?string {
 		$normalized = $this->normalizeEntry($entry);
 		if ($normalized === null) {
 			return null;
 		}
 
 		$localPath = $file->getStorage()->getLocalFile($file->getInternalPath());
 		if (!is_string($localPath) || $localPath === '') {
-			return $this->readEntryFromStream($file, $normalized);
+			return $this->readEntryFromStream($file, $normalized, $maxBytes);
 		}
 
 		$zip = new ZipArchive();
@@ -54,10 +62,10 @@ public function readEntry(File $file, string $entry): ?string {
 			// Playground filesystem) expose a nominal local path that native
 			// ZipArchive still cannot open. Treat it like any other non-local
 			// storage and retry through the portable File::fopen() path.
-			return $this->readEntryFromStream($file, $normalized);
+			return $this->readEntryFromStream($file, $normalized, $maxBytes);
 		}
 		try {
-			return $this->extractEntry($zip, $normalized);
+			return $this->extractEntry($zip, $normalized, $maxBytes);
 		} finally {
 			$zip->close();
 		}
@@ -68,7 +76,7 @@ public function readEntry(File $file, string $entry): ?string {
 	 * the stream fallback go through here so they enforce identical limits
 	 * and agree on returning null for missing entries.
 	 */
-	private function extractEntry(ZipArchive $zip, string $entry): ?string {
+	private function extractEntry(ZipArchive $zip, string $entry, ?int $maxBytes): ?string {
 		if ($zip->numFiles > $this->maxEntries) {
 			return null;
 		}
@@ -77,7 +85,7 @@ private function extractEntry(ZipArchive $zip, string $entry): ?string {
 			return null;
 		}
 		$declaredSize = $stat['size'];
-		if ($declaredSize > $this->maxUncompressedSizeBytes) {
+		if ($declaredSize > min($this->maxUncompressedSizeBytes, $maxBytes ?? PHP_INT_MAX)) {
 			throw new RuntimeException('Uncompressed entry exceeds limit');
 		}
 		// getFromName() allocates its whole $len buffer up front, so the read
@@ -161,7 +169,7 @@ public function normalizeEntry(string $entry): ?string {
 	 * Fallback for storages that cannot expose a local file (object storage,
 	 * external mounts). Copies the package to a temp file first.
 	 */
-	private function readEntryFromStream(File $file, string $entry): ?string {
+	private function readEntryFromStream(File $file, string $entry, ?int $maxBytes): ?string {
 		$tmp = tempnam(sys_get_temp_dir(), 'elpx_');
 		if ($tmp === false) {
 			return null;
@@ -183,7 +191,7 @@ private function readEntryFromStream(File $file, string $entry): ?string {
 				return null;
 			}
 			try {
-				return $this->extractEntry($zip, $entry);
+				return $this->extractEntry($zip, $entry, $maxBytes);
 			} finally {
 				$zip->close();
 			}

tests/Unit/Controller/AssetControllerTest.php

@@ -17,18 +17,20 @@
 use PHPUnit\Framework\TestCase;
 
 final class AssetControllerTest extends TestCase {
+	private IRequest $request;
 	private IUserSession $userSession;
 	private ElpxPackageService $packages;
 	private ZipEntryService $zipEntries;
 	private AssetController $controller;
 
 	protected function setUp(): void {
+		$this->request = $this->createMock(IRequest::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->packages = $this->createMock(ElpxPackageService::class);
 		$this->zipEntries = $this->createMock(ZipEntryService::class);
 		$this->controller = new AssetController(
 			'exelearning',
-			$this->createMock(IRequest::class),
+			$this->request,
 			$this->userSession,
 			$this->packages,
 			$this->zipEntries,
@@ -44,6 +46,16 @@ public function testRejectsAnonymousRequests(): void {
 		self::assertSame(['error' => 'Not authenticated'], $response->getData());
 	}
 
+	public function testRefusesTopLevelNavigation(): void {
+		$this->authenticate();
+		$this->request->method('getHeader')->with('Sec-Fetch-Dest')->willReturn('document');
+		$this->packages->expects(self::never())->method('getForUserById');
+
+		$response = $this->controller->fetch('42', 'index.html');
+
+		self::assertSame(Http::STATUS_FORBIDDEN, $response->getStatus());
+	}
+
 	public function testRejectsInvalidSessionId(): void {
 		$this->authenticate();
 

tests/Unit/Controller/ThumbnailControllerTest.php

@@ -60,14 +60,24 @@ public function testReturnsNotFoundWithoutScreenshot(): void {
 		$this->authenticate();
 		$file = $this->createMock(File::class);
 		$this->packages->method('getForUserById')->willReturn($file);
-		$this->zipEntries->method('readEntry')->with($file, 'screenshot.png')->willReturn(null);
+		$this->zipEntries->method('readEntry')->with($file, 'screenshot.png', ZipEntryService::MAX_SCREENSHOT_BYTES)->willReturn(null);
 
 		$response = $this->controller->byFileId(42);
 
 		self::assertSame(Http::STATUS_NOT_FOUND, $response->getStatus());
 		self::assertSame(['error' => 'No screenshot'], $response->getData());
 	}
 
+	public function testTreatsOversizedScreenshotAsMissing(): void {
+		$this->authenticate();
+		$this->packages->method('getForUserById')->willReturn($this->createMock(File::class));
+		$this->zipEntries->method('readEntry')->willThrowException(new \RuntimeException('too big'));
+
+		$response = $this->controller->byFileId(42);
+
+		self::assertSame(Http::STATUS_NOT_FOUND, $response->getStatus());
+	}
+
 	public function testReturnsPngWithPrivateCacheHeaders(): void {
 		$this->authenticate();
 		$file = $this->createMock(File::class);

tests/Unit/Service/ZipEntryServiceTest.php

@@ -157,6 +157,15 @@ public function testLocalPathRejectsEntriesOverTheUncompressedSizeLimit(): void
 		}
 	}
 
+	public function testPerReadCapTightensTheUncompressedSizeLimit(): void {
+		$archive = $this->createTestArchive('screenshot.png', '0123456789');
+		$file = $this->createFakeFile($archive, '');
+
+		self::assertSame('0123456789', $this->service->readEntry($file, 'screenshot.png', 10));
+		$this->expectException(RuntimeException::class);
+		$this->service->readEntry($file, 'screenshot.png', 9);
+	}
+
 	public function testStreamFallbackReturnsNullWhenArchiveHasTooManyEntries(): void {
 		$service = new ZipEntryService(maxEntries: 1);
 		$archivePath = tempnam(sys_get_temp_dir(), 'elpx_test_');