From c4902c2e4aee054dc2b36f348500333aaad5b778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:27:41 +0200 Subject: [PATCH] fix(dav): honour the write hook veto on legacy chunked uploads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy WebDAV chunked upload path assembled the final file without respecting the pre-write hook result, so the filename blacklist that applies to ordinary uploads was not enforced for chunked uploads. The chunked assembly now aborts when a write hook vetoes the file, matching the non-chunked upload path. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- apps/dav/lib/Connector/Sabre/File.php | 10 ++++- .../tests/unit/Connector/Sabre/FileTest.php | 40 +++++++++++++++++++ changelog/unreleased/41762 | 9 +++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/41762 diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index b491680b205a..d79e41582de6 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -553,7 +553,15 @@ private function createFileChunked($data) { try { $this->fileView->lockFile($targetPath, ILockingProvider::LOCK_SHARED); - $this->emitPreHooks($exists, $targetPath); + $run = $this->emitPreHooks($exists, $targetPath); + if ($run === false) { + // a hook (e.g. the blacklist check on the decoded target + // name) vetoed the write - abort before assembling the + // chunks into the final file + $this->fileView->unlockFile($targetPath, ILockingProvider::LOCK_SHARED); + $chunk_handler->cleanup(); + throw new Forbidden('Writing the file was not allowed'); + } $this->fileView->changeLock($targetPath, ILockingProvider::LOCK_EXCLUSIVE); /** @var \OC\Files\Storage\Storage $targetStorage */ list($targetStorage, $targetInternalPath) = $this->fileView->resolvePath($targetPath); diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php index 29c9d923201e..737e9ccfcd4a 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -679,6 +679,46 @@ public function testChunkedPut() { $this->assertNotEmpty($this->doPut('/test.txt-chunking-12345-2-1')); } + /** + * A chunked upload whose assembled target name is blacklisted (e.g. + * `.htaccess`) must be rejected, just like the non-chunked path. The + * chunk names themselves (`.htaccess-chunking-...`) are not blacklisted, + * so the guard has to fire on the decoded target name. + */ + public function testChunkedPutToBlacklistedNameIsForbidden() { + // wire the blacklist guard exactly as lib/kernel.php does at boot + // (setUp() clears all hooks, so it is not registered by default) + \OC_Hook::connect( + 'OC_Filesystem', + 'write', + 'OC\Files\Filesystem', + 'isForbiddenFileOrDir_Hook' + ); + + $_SERVER['HTTP_OC_CHUNKED'] = true; + + $this->assertNull($this->doPut('/.htaccess-chunking-12345-2-0')); + + $thrown = false; + try { + $this->doPut('/.htaccess-chunking-12345-2-1'); + } catch (\Sabre\DAV\Exception\Forbidden $e) { + $thrown = true; + } + $this->assertTrue($thrown, 'Uploading a blacklisted .htaccess via chunking must be forbidden'); + + // The blacklisted target must never reach the storage backend - check + // the physical storage directly, because the file cache scanner skips + // blacklisted names and would hide a file that is actually on disk. + $view = Filesystem::getView(); + list($storage, $internalPath) = $view->resolvePath('/.htaccess'); + $this->assertFalse( + $storage->file_exists($internalPath), + 'Blacklisted .htaccess must not be written to storage via chunked upload' + ); + $this->assertEmpty($this->listPartFiles(), 'No stray part files'); + } + /** * Test that putting a file triggers create hooks */ diff --git a/changelog/unreleased/41762 b/changelog/unreleased/41762 new file mode 100644 index 000000000000..7f845ce5c702 --- /dev/null +++ b/changelog/unreleased/41762 @@ -0,0 +1,9 @@ +Change: Honour the write hook veto on legacy chunked WebDAV uploads + +The legacy WebDAV chunked upload path assembled the final file without +respecting the pre-write hook result, so the filename blacklist that applies +to ordinary uploads was not enforced for chunked uploads. The chunked +assembly now aborts when a write hook vetoes the file, matching the +non-chunked upload path. + +https://github.com/owncloud/core/pull/41762