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

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
pull/53666/head
Côme Chilliet 2025-07-03 11:02:06 +07:00 committed by Côme Chilliet
parent eb91798252
commit e2861b42d1
2 changed files with 39 additions and 18 deletions

@ -249,7 +249,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)) {

@ -23,6 +23,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;
@ -185,11 +186,11 @@ class Encryption extends Wrapper {
public function rename(string $source, string $target): bool {
$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);
@ -210,9 +211,9 @@ class Encryption extends Wrapper {
public function rmdir(string $path): bool {
$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);
}
@ -225,9 +226,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);
@ -384,9 +385,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
@ -622,8 +623,8 @@ class Encryption extends Wrapper {
): bool {
// 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
@ -673,12 +674,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);
}
}
@ -728,6 +733,9 @@ class Encryption extends Wrapper {
public function hash(string $type, string $path, bool $raw = false): string|false {
$fh = $this->fopen($path, 'rb');
if ($fh === false) {
return false;
}
$ctx = hash_init($type);
hash_update_stream($ctx, $fh);
fclose($fh);
@ -752,6 +760,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);
}
@ -882,6 +893,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);