fix(encryption): Correctly handle file opening and copying failures

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
This commit is contained in:
Côme Chilliet 2025-07-03 11:02:06 +02:00
parent 32782e12b5
commit 72eba38e20
No known key found for this signature in database
GPG key ID: A3E2F658B28C760A
2 changed files with 39 additions and 18 deletions

View file

@ -293,7 +293,14 @@ class EncryptAll {
$target = $path . '.encrypted.' . time();
try {
$this->rootView->copy($source, $target);
$copySuccess = $this->rootView->copy($source, $target);
if ($copySuccess === false) {
/* Copy failed, abort */
if ($this->rootView->file_exists($target)) {
$this->rootView->unlink($target);
}
throw new \Exception('Copy failed for ' . $source);
}
$this->rootView->rename($target, $source);
} catch (DecryptionFailedException $e) {
if ($this->rootView->file_exists($target)) {

View file

@ -24,6 +24,7 @@ use OCP\Encryption\IFile;
use OCP\Encryption\IManager;
use OCP\Encryption\Keys\IStorage;
use OCP\Files\Cache\ICacheEntry;
use OCP\Files\GenericFileException;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\Storage;
use Psr\Log\LoggerInterface;
@ -265,11 +266,11 @@ class Encryption extends Wrapper {
public function rename($source, $target) {
$result = $this->storage->rename($source, $target);
if ($result &&
if ($result
// versions always use the keys from the original file, so we can skip
// this step for versions
$this->isVersion($target) === false &&
$this->encryptionManager->isEnabled()) {
&& $this->isVersion($target) === false
&& $this->encryptionManager->isEnabled()) {
$sourcePath = $this->getFullPath($source);
if (!$this->util->isExcluded($sourcePath)) {
$targetPath = $this->getFullPath($target);
@ -296,9 +297,9 @@ class Encryption extends Wrapper {
public function rmdir($path) {
$result = $this->storage->rmdir($path);
$fullPath = $this->getFullPath($path);
if ($result &&
$this->util->isExcluded($fullPath) === false &&
$this->encryptionManager->isEnabled()
if ($result
&& $this->util->isExcluded($fullPath) === false
&& $this->encryptionManager->isEnabled()
) {
$this->keyStorage->deleteAllFileKeys($fullPath);
}
@ -317,9 +318,9 @@ class Encryption extends Wrapper {
$metaData = $this->getMetaData($path);
if (
!$this->is_dir($path) &&
isset($metaData['encrypted']) &&
$metaData['encrypted'] === true
!$this->is_dir($path)
&& isset($metaData['encrypted'])
&& $metaData['encrypted'] === true
) {
$fullPath = $this->getFullPath($path);
$module = $this->getEncryptionModule($path);
@ -493,9 +494,9 @@ class Encryption extends Wrapper {
$size = $this->storage->filesize($path);
$result = $unencryptedSize;
if ($unencryptedSize < 0 ||
($size > 0 && $unencryptedSize === $size) ||
$unencryptedSize > $size
if ($unencryptedSize < 0
|| ($size > 0 && $unencryptedSize === $size)
|| $unencryptedSize > $size
) {
// check if we already calculate the unencrypted size for the
// given path to avoid recursions
@ -764,8 +765,8 @@ class Encryption extends Wrapper {
) {
// for versions we have nothing to do, because versions should always use the
// key from the original file. Just create a 1:1 copy and done
if ($this->isVersion($targetInternalPath) ||
$this->isVersion($sourceInternalPath)) {
if ($this->isVersion($targetInternalPath)
|| $this->isVersion($sourceInternalPath)) {
// remember that we try to create a version so that we can detect it during
// fopen($sourceInternalPath) and by-pass the encryption in order to
// create a 1:1 copy of the file
@ -815,12 +816,16 @@ class Encryption extends Wrapper {
try {
$source = $sourceStorage->fopen($sourceInternalPath, 'r');
$target = $this->fopen($targetInternalPath, 'w');
[, $result] = \OC_Helper::streamCopy($source, $target);
if ($source === false || $target === false) {
$result = false;
} else {
[, $result] = \OC_Helper::streamCopy($source, $target);
}
} finally {
if (is_resource($source)) {
if (isset($source) && $source !== false) {
fclose($source);
}
if (is_resource($target)) {
if (isset($target) && $target !== false) {
fclose($target);
}
}
@ -870,6 +875,9 @@ class Encryption extends Wrapper {
public function hash($type, $path, $raw = false) {
$fh = $this->fopen($path, 'rb');
if ($fh === false) {
return false;
}
$ctx = hash_init($type);
hash_update_stream($ctx, $fh);
fclose($fh);
@ -897,6 +905,9 @@ class Encryption extends Wrapper {
$firstBlock = '';
if ($this->storage->is_file($path)) {
$handle = $this->storage->fopen($path, 'r');
if ($handle === false) {
return '';
}
$firstBlock = fread($handle, $this->util->getHeaderSize());
fclose($handle);
}
@ -1046,6 +1057,9 @@ class Encryption extends Wrapper {
public function writeStream(string $path, $stream, ?int $size = null): int {
// always fall back to fopen
$target = $this->fopen($path, 'w');
if ($target === false) {
throw new GenericFileException("Failed to open $path for writing");
}
[$count, $result] = \OC_Helper::streamCopy($stream, $target);
fclose($stream);
fclose($target);