Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion apps/dav/lib/Connector/Sabre/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
40 changes: 40 additions & 0 deletions apps/dav/tests/unit/Connector/Sabre/FileTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
9 changes: 9 additions & 0 deletions changelog/unreleased/41762
Original file line number Diff line number Diff line change
@@ -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