Skip to content

Commit 966765b

Browse files
salmart-devAndyScherzinger
authored andcommitted
fix: fail uploads when uri does not match session
This is a cosmetic change that can cause false security reports: the current implementation of the upload root collection returns UploadHome in every case, based on the current session, either the one of the logged in user, or for the share. The former allows uploading files in what looks like the upload folder of another user, but is in reality the one of the logged in user. Those requests will now fail with a 403 instead. Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
1 parent 8dd3101 commit 966765b

1 file changed

Lines changed: 10 additions & 1 deletion

File tree

apps/dav/lib/Upload/RootCollection.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
use OCP\Files\IRootFolder;
1414
use OCP\IUserSession;
1515
use OCP\Share\IManager;
16+
use Sabre\DAV\Exception\Forbidden;
17+
use Sabre\DAV\INode;
1618
use Sabre\DAVACL\AbstractPrincipalCollection;
1719
use Sabre\DAVACL\PrincipalBackend;
1820

@@ -34,7 +36,14 @@ public function __construct(
3436
* @inheritdoc
3537
*/
3638
#[\Override]
37-
public function getChildForPrincipal(array $principalInfo): UploadHome {
39+
public function getChildForPrincipal(array $principalInfo): INode|UploadHome {
40+
[$prefix, $name] = \Sabre\Uri\split($principalInfo['uri']);
41+
$user = $this->userSession->getUser();
42+
if ($prefix !== 'principals/shares' && $user?->getUID() !== $name) {
43+
// if the request is not using a share token and the URL does not match the user, error out
44+
throw new Forbidden('Not allowed');
45+
}
46+
3847
return new UploadHome(
3948
$principalInfo,
4049
$this->cleanupService,

0 commit comments

Comments
 (0)