Skip to content

Commit 4427050

Browse files
committed
fix(encryption): Correctly handle file opening and copying failures
Signed-off-by: Côme Chilliet <[email protected]>
1 parent 58e8626 commit 4427050

File tree

2 files changed

+25
-4
lines changed

2 files changed

+25
-4
lines changed

apps/encryption/lib/Crypto/EncryptAll.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,14 @@ protected function encryptFile($path) {
249249
$target = $path . '.encrypted.' . time();
250250

251251
try {
252-
$this->rootView->copy($source, $target);
252+
$copySuccess = $this->rootView->copy($source, $target);
253+
if ($copySuccess === false) {
254+
/* Copy failed, abort */
255+
if ($this->rootView->file_exists($target)) {
256+
$this->rootView->unlink($target);
257+
}
258+
throw new \Exception('Copy failed for ' . $source);
259+
}
253260
$this->rootView->rename($target, $source);
254261
} catch (DecryptionFailedException $e) {
255262
if ($this->rootView->file_exists($target)) {

lib/private/Files/Storage/Wrapper/Encryption.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use OCP\Encryption\Keys\IStorage;
2424
use OCP\Files;
2525
use OCP\Files\Cache\ICacheEntry;
26+
use OCP\Files\GenericFileException;
2627
use OCP\Files\Mount\IMountPoint;
2728
use OCP\Files\Storage;
2829
use Psr\Log\LoggerInterface;
@@ -685,12 +686,16 @@ private function copyBetweenStorage(
685686
try {
686687
$source = $sourceStorage->fopen($sourceInternalPath, 'r');
687688
$target = $this->fopen($targetInternalPath, 'w');
688-
[, $result] = Files::streamCopy($source, $target, true);
689+
if ($source === false || $target === false) {
690+
$result = false;
691+
} else {
692+
[, $result] = Files::streamCopy($source, $target, true);
693+
}
689694
} finally {
690-
if (is_resource($source)) {
695+
if (isset($source) && $source !== false) {
691696
fclose($source);
692697
}
693-
if (is_resource($target)) {
698+
if (isset($target) && $target !== false) {
694699
fclose($target);
695700
}
696701
}
@@ -740,6 +745,9 @@ public function stat(string $path): array|false {
740745

741746
public function hash(string $type, string $path, bool $raw = false): string|false {
742747
$fh = $this->fopen($path, 'rb');
748+
if ($fh === false) {
749+
return false;
750+
}
743751
$ctx = hash_init($type);
744752
hash_update_stream($ctx, $fh);
745753
fclose($fh);
@@ -764,6 +772,9 @@ protected function readFirstBlock(string $path): string {
764772
$firstBlock = '';
765773
if ($this->storage->is_file($path)) {
766774
$handle = $this->storage->fopen($path, 'r');
775+
if ($handle === false) {
776+
return '';
777+
}
767778
$firstBlock = fread($handle, $this->util->getHeaderSize());
768779
fclose($handle);
769780
}
@@ -894,6 +905,9 @@ protected function shouldEncrypt(string $path): bool {
894905
public function writeStream(string $path, $stream, ?int $size = null): int {
895906
// always fall back to fopen
896907
$target = $this->fopen($path, 'w');
908+
if ($target === false) {
909+
throw new GenericFileException("Failed to open $path for writing");
910+
}
897911
[$count, $result] = Files::streamCopy($stream, $target, true);
898912
fclose($stream);
899913
fclose($target);

0 commit comments

Comments
 (0)