Leaking deleted file content when renaming group folder paths

[melegiul]

How to use GitHub

• Please use the 👍 reaction to show that you are affected by the same issue.
• Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
• Subscribe to receive notifications on status change and new comments.

Might be related to https://github.com/nextcloud/groupfolders/issues/2626

Steps to reproduce

1. Create a group folder /groupfolder
2. Create a sub folder /groupfolder/sub
3. Create a file in the sub folder /groupfolder/sub/to-be-deleted
4. Delete the file /groupfolder/sub/to-be-deleted
5. See that only people with granted access can download the file
6. Now rename the folder to /groupfolder/sub-edited
7. See that all people which are members of the group folder can download file from trash bin

Expected behaviour

After renaming a folder inside a group folder, all previously deleted files should keep their ACL rules. People without permission should not be able to download these files.

Actual behaviour

After renameing the folder inside a group folder, all files in trash bin originating from that folder can be downloaded from members of the group folder. ACLs from its parent folder are ignored.

Server configuration

Operating system: debian

Web server: nginx

Database: mariadb

PHP version: 8.1

Nextcloud version: 27.1.7

Group folders version: 15.3.5

Updated from an older Nextcloud/ownCloud or fresh install: Fresh install

Where did you install Nextcloud from: Nextcloud .tar archive

Are you using external storage, if yes which one: no

Are you using encryption: no

Are you using an external user-backend, if yes which one: Saml

Client configuration

Browser: 115.8.0esr Firefox

Operating system: Debian 12

Logs

Nextcloud log (data/nextcloud.log)

This log shows only up when trying to permanently delete the file. Something similar should happen when trying to download.

Nextcloud log

{"reqId":"RE3tEggY0VK8wagTsSIx","level":3,"time":"2024-03-18T16:46:21+00:00","remoteAddr":"2a01:4f9:c011:e8a::1","user":
"redacted","app":"webdav","method":"DELETE","url":"/remote.php/dav/trashbin/redactedtrash/test-trash.md.d1710779511",
"message":"Exception thrown: OCP\\Files\\NotPermittedException","userAgent":"Mozilla/5.0 (X11; Linux x86_64; rv:109.0)
Gecko/20100101 Firefox/115.0","version":"27.1.7.2","exception":{"Exception":"OCP\\Files\\NotPermittedException", "Message":"","Code":0,"Trace":[{"file":"/opt/nextcloud-27.1.7/apps/files_trashbin/lib/Trash/TrashManager.php", "line":68,"function":"removeItem","class":"OCA\\GroupFolders\\Trash\\TrashBackend","type":"->"},{"file":"/opt/nextcloud-27.1.7/apps/files_trashbin/lib/Sabre/AbstractTrash.php","line":93,"function":"removeItem","class":"OCA\\Files_Trashbin\\Trash\\TrashManager","type":"->"},
{"file":"/opt/nextcloud-27.1.7/3rdparty/sabre/dav/lib/DAV/Tree.php","line":179,"function":"delete","class":"OCA\\Files_Trashbin\\Sabre\\AbstractTrash","type":"->"},
{"file":"/opt/nextcloud-27.1.7/3rdparty/sabre/dav/lib/DAV/CorePlugin.php","line":281,"function":"delete","class":"Sabre\\DAV\\Tree","type":"->"},
{"file":"/opt/nextcloud-27.1.7/3rdparty/sabre/even/lib/WildcardEmitterTrait.php","line":89,"function":"httpDelete","class":"Sabre\\DAV\\CorePlugin","type":"->"},
{"file":"/opt/nextcloud-27.1.7/3rdparty/sabre/dav/lib/DAV/Server.php","line":472,"function":"emit","class":"Sabre\\DAV\\Server","type":"->"},{"file":"/opt/nextcloud-27.1.7/3rdparty/sabre/dav/lib/DAV/Server.php","line":253,"function":"invokeMethod","class":"Sabre\\DAV\\Server","type":"->"},{"file":"/opt/nextcloud-27.1.7/3rdparty/sabre/dav/lib/DAV/Server.php","line":321,"function":"start","class":"Sabre\\DAV\\Server","type":"->"},{"file":"/opt/nextcloud-27.1.7/apps/dav/lib/Server.php","line":368,"function":"exec","class":"Sabre\\DAV\\Server","type":"->"},{"file":"/opt/nextcloud-27.1.7/apps/dav/appinfo/v2/remote.php","line":35,"function":"exec","class":"OCA\\DAV\\Server","type":"->"},{"file":"/opt/nextcloud-27.1.7/remote.php","line":172,"args":["/opt/nextcloud-27.1.7/apps/dav/appinfo/v2/remote.php"],"function":"require_once"}],"File":"/opt/nextcloud-27.1.7/apps/groupfolders/lib/Trash/TrashBackend.php","Line":204,"message":"","exception":{},"CustomMessage":"Exception thrown: OCP\\Files\\NotPermittedException"}}

[svenseeberg]

One addition to the steps to reproduce:

1. Create a group folder /groupfolder
2. Create a sub folder /groupfolder/sub
3. Use ACLs to grant access to /groupfolder/sub for user A and deny access to for user B
4. Create a file in the sub folder /groupfolder/sub/to-be-deleted
5. Delete the file /groupfolder/sub/to-be-deleted
6. See that only user A can download the file from the trash bin
7. Now rename the folder to /groupfolder/sub-edited
8. See that user B can download file from trash bin

[come-nc]

Should have been fixed by https://github.com/nextcloud/groupfolders/pull/2713

[come-nc]

https://github.com/nextcloud/groupfolders/blob/master/cypress/e2e/groupfolders.cy.ts#L242 On master it’s tested.

I think the problem is https://github.com/nextcloud/groupfolders/pull/2631 was not backported to stable27

[come-nc]

https://github.com/nextcloud/groupfolders/pull/2756 Actually it was backported as well…

@svenseeberg Could you try to reproduce on newer versions to know if this is specific to 27 or not?

[svenseeberg]

Should have been fixed by #2713

Are you sure? Because there is a difference AFAICT: #2713 is about applying ACLs to deleted items. This is not the problem here. The problem is about the (not trashed) parent directory being renamed after a file has been trashed.

*edit: https://github.com/nextcloud/groupfolders/pull/2713/files#diff-6b183c35236437b385541adf0a1ca3dc67d4462a6241c057c0ea4ceb9dcb7503R84 should probably fix the issue. I'll try to debug further.

[svenseeberg]

@svenseeberg Could you try to reproduce on newer versions to know if this is specific to 27 or not?

I cannot reproduce it with NC 28.

[come-nc]

For me it should be fixed by #2713 and #2756 yes. If it’s fixed on 28 we should look into differences between versions on server and app sides. Maybe the fix is from another PR which was not backported?

[svenseeberg]

It is definitely fixed in NC 28 and NC 27 is nearing its EOL. We upgraded to 28 therefore it is no longer an issue for us. I'm in favor of closing the issue.

[provokateurin]

Closing as it is fixed on all supported versions.