From 094e74031b3e32115167ed7180755138c7c1cc27 Mon Sep 17 00:00:00 2001 From: Maximilian von Heyden Date: Sat, 19 Sep 2026 10:34:53 +0200 Subject: [PATCH] fix(encryption): apply the file header before reading the unencrypted block size fixUnencryptedSize() asked the encryption module for the unencrypted block size before calling begin(). The module only learns a file's encoding (binary or legacy base64) from the header in begin(), so it answered from its default and returned the binary block size for every file. For legacy base64 files that is 8096 instead of 6072 bytes per block, and the recalculated unencrypted_size ends up exactly 4/3 too large. The wrong value breaks downloads (Content-Length larger than the data), shows files as 0 B when the recalculation cannot run, and is never corrected again because it stays below the on-disk size and so passes every plausibility check in verifyUnencryptedSize(). Since the position suffix "end" for the last block is derived from unencrypted_size, the inflated value also makes the last block fail its signature check. begin() only resolves file key, cipher, version and encoding and does not touch the stream, so it can be called before the block size is read. Fixes #63662 Signed-off-by: Maximilian von Heyden Co-Authored-By: Claude Fable 5.1 --- .../Files/Storage/Wrapper/Encryption.php | 8 +- .../Files/Storage/Wrapper/EncryptionTest.php | 83 +++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index d358e660f1db5..4bfc1ecab3572 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -477,6 +477,13 @@ protected function fixUnencryptedSize(string $path, int $size, int $unencryptedS } $signed = isset($header['signed']) && $header['signed'] === 'true'; + + // The module only knows the file's encoding (binary or legacy base64) + // once begin() has parsed the header. Asking for the block size before + // that returns the module default, which is wrong for legacy files and + // inflates the result by 4/3 (8096 instead of 6072 bytes per block). + // begin() does not touch the stream, so it is safe to call it here. + $encryptionModule->begin($this->getFullPath($path), $this->uid, 'r', $header, []); $unencryptedBlockSize = $encryptionModule->getUnencryptedBlockSize($signed); // calculate last chunk nr @@ -508,7 +515,6 @@ protected function fixUnencryptedSize(string $path, int $size, int $unencryptedS fclose($stream); // we have to decrypt the last chunk to get it actual size - $encryptionModule->begin($this->getFullPath($path), $this->uid, 'r', $header, []); $decryptedLastChunk = $encryptionModule->decrypt($lastChunkContentEncrypted, $lastChunkNr . 'end'); $decryptedLastChunk .= $encryptionModule->end($this->getFullPath($path), $lastChunkNr . 'end'); diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php index 0e22db203d1ce..b56c99fb4d3fa 100644 --- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php @@ -608,6 +608,89 @@ public static function dataTestVerifyUnencryptedSize(): array { ]; } + /** + * Legacy base64 files only reveal their encoding through the header, which + * the module applies in begin(). The block size must therefore be read + * after begin(); otherwise the module default (binary) is used and the + * result is inflated by 4/3. + */ + public function testFixUnencryptedSizeUsesBlockSizeFromHeader(): void { + $fullBlocks = 3; + $lastBlockCiphertext = str_repeat('c', 500); + $lastBlockPlaintext = str_repeat('p', 100); + $physicalSize = $this->headerSize + $fullBlocks * 8192 + strlen($lastBlockCiphertext); + + $stream = fopen('php://memory', 'r+'); + fwrite($stream, str_repeat('h', $this->headerSize)); + fwrite($stream, str_repeat('b', $fullBlocks * 8192)); + fwrite($stream, $lastBlockCiphertext); + rewind($stream); + + // The module learns the encoding in begin(): before that it answers + // with the binary block size, afterwards with the legacy one. + $headerApplied = false; + $module = $this->createMock(IEncryptionModule::class); + $module->expects($this->once())->method('begin') + ->willReturnCallback(function () use (&$headerApplied) { + $headerApplied = true; + return []; + }); + $module->expects($this->any())->method('getUnencryptedBlockSize') + ->willReturnCallback(function () use (&$headerApplied) { + return $headerApplied ? 6072 : 8096; + }); + $module->expects($this->once())->method('decrypt') + ->with($lastBlockCiphertext, $fullBlocks . 'end') + ->willReturn($lastBlockPlaintext); + $module->expects($this->any())->method('end')->willReturn(''); + + $expected = $fullBlocks * 6072 + strlen($lastBlockPlaintext); + + $cache = $this->createMock(ICache::class); + $cache->expects($this->any())->method('get')->willReturn(['fileid' => 42]); + $cache->expects($this->once())->method('update') + ->with(42, ['unencrypted_size' => $expected]); + + $sourceStorage = $this->getMockBuilder('\OC\Files\Storage\Storage') + ->disableOriginalConstructor()->getMock(); + $sourceStorage->expects($this->once())->method('fopen') + ->with('/legacy.txt', 'r') + ->willReturn($stream); + $sourceStorage->expects($this->any())->method('getCache')->willReturn($cache); + + $instance = $this->getMockBuilder(Encryption::class) + ->setConstructorArgs( + [ + [ + 'storage' => $sourceStorage, + 'root' => 'foo', + 'mountPoint' => '/', + 'mount' => $this->mount + ], + $this->encryptionManager, + $this->util, + $this->logger, + $this->file, + null, + $this->keyStore, + $this->mountManager, + $this->arrayCache, + ] + ) + ->onlyMethods(['getHeader', 'getHeaderSize', 'getEncryptionModule']) + ->getMock(); + $instance->expects($this->any())->method('getHeaderSize')->willReturn($this->headerSize); + $instance->expects($this->any())->method('getHeader') + ->willReturn(['signed' => 'true', 'oc_encryption_module' => 'OC_DEFAULT_MODULE']); + $instance->expects($this->any())->method('getEncryptionModule')->willReturn($module); + + // int|float: the chunk arithmetic goes through ceil(), so the result is a float. + $this->assertEquals( + $expected, + $this->invokePrivate($instance, 'fixUnencryptedSize', ['/legacy.txt', $physicalSize, 0]) + ); + } + /** * * @param string $source