Users can Transfer ownership of a file or folder even if excluded from sharing

[forsequans]

Hello,

We have restricted some accounts to access file sharing by making those members of groups excluded from sharing with option "Exclude groups from sharing" in administration settings/sharing.
So those users can't share files, and by the way can't view existing accounts.

Since we moved from NX15 to NX18, a new features has appeared in personal settings/sharing: "Transfer ownership of a file or folder".
This is a nice feature, but it is not only very partially obey to administration settings/sharing parameters.
If it obey to "Allow username autocompletion in share dialog", it doesn't to all other parameters like "Restrict users to only share with users in their groups" or "Exclude groups from sharing".

Because of that, a user who was not allowed to use sharing now is allowed to transfer files and folders to another user which is a form of sharing bypassing the sharing rules.
This user can also find all other existing accounts, if "Allow username autocompletion in share dialog" or just by typing the username if it can guess how usernames are built.

This has big privacy impact and is a real security mess

[kesselb]

cc @rullzer @ChristophWurst

[ChristophWurst]

To anyone who would like to work on this

• Here is the controller responsible for the user interactions https://github.com/nextcloud/server/blob/master/apps/files/lib/Controller/TransferOwnershipController.php
• Here is the service that processes the move https://github.com/nextcloud/server/blob/master/apps/files/lib/Service/OwnershipTransferService.php

[Tetrachloromethane250]

Hello,

TLDR: ShareesAPIController should return an empty list if the person asking for who they're allowed to share with is only in groups that are excluded from sharing. Related PR: #25744

Just come across this problem myself. After a bit of thought I decided that it wasn't the Transfer Ownership implementation that was at fault, but the search for possible sharees which is a GET request routed to ShareesAPIController:search(). If the user gets no options in the "New Owner" dialog then it fixes both the ability to bypass the sharing restriction and the security concern of being able to see all users on the server.
(https://github.com/nextcloud/server/blob/stable20/apps/files/src/components/TransferOwnershipDialogue.vue#L165, https://github.com/nextcloud/server/blob/stable20/apps/files_sharing/lib/Controller/ShareesAPIController.php#L140)

The search function in the controller for this does not check whether a user is allowed to share or not - presumably assuming that the ability to call it is limited at a higher level (searching for calls to this function, nothing server-side includes ShareesAPIController and the only Axios calls to it that I can find are in places where this check would already have been performed). The search call is passed down by the ShareesAPIController until it is resolved by Collaborators/Search (https://github.com/nextcloud/server/blob/stable20/lib/private/Collaboration/Collaborators/Search.php), where each individual search plugin is responsible for checking sharing restriction (such as sharing within groups only).

To me this suggests there are two ways to fix this - either modify the TransfewOwnerShipDialog to include some check that a user is allowed to share before allowing them to search for users, or modify somewhere along the server-side search progression to check that a user is allowed to share before resolving their search. If this slipped by then it seems likely that future changes could easily open this issue back up but with a different entry point - it seems like it would be better to fix the search than patch over the entry point to it.

This does lead to the slight issue of 'What exactly does preventing a user from sharing prevent' - adding the check too far down the hierarchy of the search could prevent some intended behaviour in other apps, if users that are restricted from sharing should still be able to find other users for example. I would argue that someone who is not allowed to share would definitely have no "Sharees" but may still have "Collaborators", so the ShareesAPIController should perform the check and return an empty response without passing the request to the collaborator search. A related issue is 'If a user is part of a disallowed group and an allowed group, should they be allowed to share?'. This is a tad more complicated, but I'd tend to go with the Share Manager (https://github.com/nextcloud/server/blob/stable20/lib/private/Share20/Manager.php#L1850) and say yes - if this allowed group was 'admin' for example, I would definitely want them to be able to share, and trying to add granularity to have groups which can always share even if part of a blacklisted group seems overly complex.

So the overall solution seems to be that ShareesAPIController should check the requesting user's groups, and if they are only part of groups that are excluded from sharing then it should return an empty response without performing the search (in the same way as it does if the search term is too short).

[szaimen]

Hi, please update to 24.0.8 or better 25.0.2 and report back if it fixes the issue. Thank you!

[Kontergewicht]

Is this problem really solved? I have the same issue with Version 27.1.3. I have multiple groups which are excluded from sharing, but in their personal setting, they are able to transfer the ownership of their files and folders. But much worse is the fact, that they get shown the whole global adressbook. This is GDPR-wise a huge mess. Is this problem solveabe except to disable the auto-fill for the adressbook?