Skip to content

Commit b32c0f7

Browse files
authored
Merge pull request #39990 from nextcloud/recursive-share
add some recrusive detection/prevention
2 parents 5a099a9 + d1c6e82 commit b32c0f7

3 files changed

Lines changed: 48 additions & 8 deletions

File tree

apps/files_external/tests/Storage/SmbTest.php

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ public function testNotifyGetChanges() {
105105
$this->instance->unlink('/renamed.txt');
106106
sleep(1); //time for all changes to be processed
107107

108+
/** @var IChange[] $changes */
108109
$changes = [];
109110
$count = 0;
110111
// wait up to 10 seconds for incoming changes
@@ -115,14 +116,23 @@ public function testNotifyGetChanges() {
115116
}
116117
$notifyHandler->stop();
117118

118-
$expected = [
119-
new Change(IChange::ADDED, 'newfile.txt'),
120-
new RenameChange(IChange::RENAMED, 'newfile.txt', 'renamed.txt'),
121-
new Change(IChange::REMOVED, 'renamed.txt')
122-
];
119+
// depending on the server environment, the initial create might be detected as a change instead
120+
if ($changes[0]->getType() === IChange::MODIFIED) {
121+
$expected = [
122+
new Change(IChange::MODIFIED, 'newfile.txt'),
123+
new RenameChange(IChange::RENAMED, 'newfile.txt', 'renamed.txt'),
124+
new Change(IChange::REMOVED, 'renamed.txt')
125+
];
126+
} else {
127+
$expected = [
128+
new Change(IChange::ADDED, 'newfile.txt'),
129+
new RenameChange(IChange::RENAMED, 'newfile.txt', 'renamed.txt'),
130+
new Change(IChange::REMOVED, 'renamed.txt')
131+
];
132+
}
123133

124134
foreach ($expected as $expectedChange) {
125-
$this->assertTrue(in_array($expectedChange, $changes), 'Actual changes are:' . PHP_EOL . print_r($changes, true) . PHP_EOL . 'Expected to find: ' . PHP_EOL . print_r($expectedChange, true));
135+
$this->assertTrue(in_array($expectedChange, $changes), "Expected changes are:\n" . print_r($expected, true) . PHP_EOL . 'Expected to find: ' . PHP_EOL . print_r($expectedChange, true) . "\nGot:\n" . print_r($changes, true));
126136
}
127137
}
128138

@@ -141,7 +151,12 @@ public function testNotifyListen() {
141151
return false;//stop listening
142152
});
143153

144-
$this->assertEquals(new Change(IChange::ADDED, 'newfile.txt'), $result);
154+
// depending on the server environment, the initial create might be detected as a change instead
155+
if ($result->getType() === IChange::ADDED) {
156+
$this->assertEquals(new Change(IChange::ADDED, 'newfile.txt'), $result);
157+
} else {
158+
$this->assertEquals(new Change(IChange::MODIFIED, 'newfile.txt'), $result);
159+
}
145160
}
146161

147162
public function testRenameRoot() {

apps/files_sharing/lib/SharedStorage.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
use OC\Files\Storage\FailedStorage;
4242
use OC\Files\Storage\Home;
4343
use OC\Files\Storage\Wrapper\PermissionsMask;
44+
use OC\Files\Storage\Wrapper\Wrapper;
4445
use OC\User\NoUserException;
4546
use OCA\Files_External\Config\ConfigAdapter;
4647
use OCP\Constants;
@@ -97,6 +98,8 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto
9798

9899
private string $sourcePath = '';
99100

101+
private static int $initDepth = 0;
102+
100103
public function __construct($arguments) {
101104
$this->ownerView = $arguments['ownerView'];
102105
$this->logger = \OC::$server->get(LoggerInterface::class);
@@ -136,8 +139,15 @@ private function init() {
136139
if ($this->initialized) {
137140
return;
138141
}
142+
139143
$this->initialized = true;
144+
self::$initDepth++;
145+
140146
try {
147+
if (self::$initDepth > 10) {
148+
throw new \Exception("Maximum share depth reached");
149+
}
150+
141151
/** @var IRootFolder $rootFolder */
142152
$rootFolder = \OC::$server->get(IRootFolder::class);
143153
$this->ownerUserFolder = $rootFolder->getUserFolder($this->superShare->getShareOwner());
@@ -148,6 +158,9 @@ private function init() {
148158
$this->cache = new FailedCache();
149159
$this->rootPath = '';
150160
} else {
161+
if ($this->nonMaskedStorage instanceof Wrapper && $this->nonMaskedStorage->isWrapperOf($this)) {
162+
throw new \Exception('recursive share detected');
163+
}
151164
$this->nonMaskedStorage = $ownerNode->getStorage();
152165
$this->sourcePath = $ownerNode->getPath();
153166
$this->rootPath = $ownerNode->getInternalPath();
@@ -176,6 +189,7 @@ private function init() {
176189
if (!$this->nonMaskedStorage) {
177190
$this->nonMaskedStorage = $this->storage;
178191
}
192+
self::$initDepth--;
179193
}
180194

181195
/**
@@ -409,7 +423,7 @@ public function getCache($path = '', $storage = null) {
409423
return new FailedCache();
410424
}
411425

412-
$this->cache = new Cache(
426+
$this->cache = new \OCA\Files_Sharing\Cache(
413427
$storage,
414428
$sourceRoot,
415429
\OC::$server->get(CacheDependencies::class),

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,4 +654,15 @@ public function writeStream(string $path, $stream, int $size = null): int {
654654
public function getDirectoryContent($directory): \Traversable {
655655
return $this->getWrapperStorage()->getDirectoryContent($directory);
656656
}
657+
658+
public function isWrapperOf(IStorage $storage) {
659+
$wrapped = $this->getWrapperStorage();
660+
if ($wrapped === $storage) {
661+
return true;
662+
}
663+
if ($wrapped instanceof Wrapper) {
664+
return $wrapped->isWrapperOf($storage);
665+
}
666+
return false;
667+
}
657668
}

0 commit comments

Comments
 (0)