Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable31] fix: Harden files scanner for invalid null access #50508

Merged
merged 1 commit into from
Jan 28, 2025
Merged

[stable31] fix: Harden files scanner for invalid null access #50508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions apps/files_sharing/lib/Cache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ protected function getRoot() {
/** @var Jail $currentStorage */
$absoluteRoot = $currentStorage->getJailedPath($absoluteRoot);
}
$this->root = $absoluteRoot;
$this->root = $absoluteRoot ?? '';
}
return $this->root;
}

protected function getGetUnjailedRoot() {
protected function getGetUnjailedRoot(): string {
return $this->sourceRootInfo->getPath();
}

Expand Down
5 changes: 3 additions & 2 deletions apps/files_sharing/lib/External/Scanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $loc
* @param string $file file to scan
* @param int $reuseExisting
* @param int $parentId
* @param array | null $cacheData existing data in the cache for the file to be scanned
* @param \OC\Files\Cache\CacheEntry|array|null|false $cacheData existing data in the cache for the file to be scanned
* @param bool $lock set to false to disable getting an additional read lock during scanning
* @return array | null an array of metadata of the scanned file
* @param array|null $data the metadata for the file, as returned by the storage
* @return array|null an array of metadata of the scanned file
*/
public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) {
try {
Expand Down
2 changes: 1 addition & 1 deletion apps/files_sharing/lib/Scanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData =
$sourceScanner = $this->getSourceScanner();
if ($sourceScanner instanceof ObjectStoreScanner) {
// ObjectStoreScanner doesn't scan
return [];
return null;
} else {
return parent::scanFile($file, $reuseExisting, $parentId, $cacheData, $lock);
}
Expand Down
15 changes: 0 additions & 15 deletions build/psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -873,11 +873,6 @@
<code><![CDATA[Mount]]></code>
</MoreSpecificReturnType>
</file>
<file src="apps/files_sharing/lib/External/Scanner.php">
<MoreSpecificImplementedParamType>
<code><![CDATA[$cacheData]]></code>
</MoreSpecificImplementedParamType>
</file>
<file src="apps/files_sharing/lib/MountProvider.php">
<RedundantFunctionCall>
<code><![CDATA[array_values]]></code>
Expand Down Expand Up @@ -1758,14 +1753,10 @@
</MoreSpecificReturnType>
</file>
<file src="lib/private/Files/Cache/Cache.php">
<InvalidArgument>
<code><![CDATA[$parentData]]></code>
</InvalidArgument>
<InvalidNullableReturnType>
<code><![CDATA[array]]></code>
</InvalidNullableReturnType>
<InvalidScalarArgument>
<code><![CDATA[$path]]></code>
<code><![CDATA[\OC_Util::normalizeUnicode($path)]]></code>
</InvalidScalarArgument>
<NullableReturnStatement>
Expand Down Expand Up @@ -1796,12 +1787,6 @@
<InvalidArgument>
<code><![CDATA[self::SCAN_RECURSIVE_INCOMPLETE]]></code>
</InvalidArgument>
<InvalidReturnStatement>
<code><![CDATA[$existingChildren]]></code>
</InvalidReturnStatement>
<InvalidReturnType>
<code><![CDATA[array[]]]></code>
</InvalidReturnType>
</file>
<file src="lib/private/Files/Cache/Storage.php">
<InvalidNullableReturnType>
Expand Down
32 changes: 19 additions & 13 deletions lib/private/Files/Cache/Cache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public function getNumericStorageId() {
/**
* get the stored metadata of a file or folder
*
* @param string | int $file either the path of a file or folder or the file id for a file or folder
* @param string|int $file either the path of a file or folder or the file id for a file or folder
* @return ICacheEntry|false the cache entry as array or false if the file is not found in the cache
*/
public function get($file) {
Expand All @@ -131,15 +131,17 @@ public function get($file) {
$data = $result->fetch();
$result->closeCursor();

//merge partial data
if (!$data && is_string($file) && isset($this->partial[$file])) {
return $this->partial[$file];
} elseif (!$data) {
return $data;
} else {
if ($data !== false) {
$data['metadata'] = $metadataQuery->extractMetadata($data)->asArray();
return self::cacheEntryFromData($data, $this->mimetypeLoader);
} else {
//merge partial data
if (is_string($file) && isset($this->partial[$file])) {
return $this->partial[$file];
}
}

return false;
}

/**
Expand Down Expand Up @@ -886,19 +888,23 @@ public function searchQuery(ISearchQuery $query) {
/**
* Re-calculate the folder size and the size of all parent folders
*
* @param string|boolean $path
* @param array $data (optional) meta data of the folder
* @param array|ICacheEntry|null $data (optional) meta data of the folder
*/
public function correctFolderSize($path, $data = null, $isBackgroundScan = false) {
public function correctFolderSize(string $path, $data = null, bool $isBackgroundScan = false): void {
$this->calculateFolderSize($path, $data);

if ($path !== '') {
$parent = dirname($path);
if ($parent === '.' || $parent === '/') {
$parent = '';
}

if ($isBackgroundScan) {
$parentData = $this->get($parent);
if ($parentData['size'] !== -1 && $this->getIncompleteChildrenCount($parentData['fileid']) === 0) {
if ($parentData !== false
&& $parentData['size'] !== -1
&& $this->getIncompleteChildrenCount($parentData['fileid']) === 0
) {
$this->correctFolderSize($parent, $parentData, $isBackgroundScan);
}
} else {
Expand Down Expand Up @@ -1009,8 +1015,8 @@ protected function calculateFolderSizeInner(string $path, $entry = null, bool $i
}

// only set unencrypted size for a folder if any child entries have it set, or the folder is empty
$shouldWriteUnEncryptedSize = $unencryptedMax > 0 || $totalSize === 0 || $entry['unencrypted_size'] > 0;
if ($entry['size'] !== $totalSize || ($entry['unencrypted_size'] !== $unencryptedTotal && $shouldWriteUnEncryptedSize)) {
$shouldWriteUnEncryptedSize = $unencryptedMax > 0 || $totalSize === 0 || ($entry['unencrypted_size'] ?? 0) > 0;
if ($entry['size'] !== $totalSize || (($entry['unencrypted_size'] ?? 0) !== $unencryptedTotal && $shouldWriteUnEncryptedSize)) {
if ($shouldWriteUnEncryptedSize) {
// if all children have an unencrypted size of 0, just set the folder unencrypted size to 0 instead of summing the sizes
if ($unencryptedMax === 0) {
Expand Down
Loading
Loading