desktop icon indicating copy to clipboard operation
desktop copied to clipboard

[Bug]: Folder permissions are changed during synchronization (3.13.1 & 3.13.2)

Open TopMax opened this issue 1 year ago • 25 comments

⚠️ Before submitting, please verify the following: ⚠️

Bug description

After updating from 3.13.0 to 3.13.1, this error occurs.

If a file is updated in an existing folder, the permissions are modified for all folders directly above it up to the sync root folder, the folders become writable for groups and others.

Another way is to simply create a new folder for example in the sync root folder. Directly after creation, the folder then has the following rights, for example drwx------ 2 user user 4096 Jun 30 19:14 test. After the folder was synchronized, the rights have now changed, in this example drwx-w--w- 2 user user 4096 Jun 30 19:14 test.

There are no log entries for either the server or the client.

After downgrading from 3.13.1 to 3.13.0, this error no longer occurs.

Steps to reproduce

  1. Create a new folder with only read, write and execute permission for the user.
  2. After the synchronization is complete check the permissions
  3. The permissions changed to allow the group and other to write to the folder

  1. Create a new file in a previously created folder (older client versions) and check that this folder only has read, write and execute permission for the user.
  2. Create a new file in this folder or modify an existing one.
  3. After the synchronization is complete check the permissions of the folder
  4. The permissions changed to allow the group and other to write to the folder

Expected behavior

After the synchronization no folder permissions are changed.

Which files are affected by this bug

custom created test folder

Operating system

Linux & macOS

Which version of the operating system you are running.

Arch Linux & Debian 12

Package

Distro package manager & AppImage

Nextcloud Server version

29.0.3

Nextcloud Desktop Client version

3.13.1 & 3.13.2

Is this bug present after an update or on a fresh install?

Updated from a minor version (ex. 3.4.2 to 3.4.4)

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

Are you using an external user-backend?

  • [x] Default internal user-backend
  • [ ] LDAP/ Active Directory
  • [ ] SSO - SAML
  • [ ] Other

Nextcloud Server logs

No response

Additional info

No response

TopMax avatar Jun 30 '24 17:06 TopMax

I am experiencing exactly the same.

Maybe this is caused by the the following change:

https://github.com/nextcloud/desktop/pull/6853/files#diff-a0c50fd8b42d2ffff94466bbbf73b51fcd4aa01e607c278682a53e520bcf8c96L460-R459

where writePerms equals to "owner", "group" and "others" write permssion?

ltoenning avatar Jul 02 '24 18:07 ltoenning

Yes, I can confirm this behavior. That one just messed up some of my directory and file permissions.

Finn10111 avatar Jul 07 '24 20:07 Finn10111

+1. Same behavior with macOS client.

tlebars avatar Jul 09 '24 06:07 tlebars

I have the same issue on Arch. When I create new folders locally or on the instance they change permissions to 777.

wwaleszcz avatar Jul 10 '24 16:07 wwaleszcz

+1 same here behavior with macOS client

jay-schulz-openfellas avatar Jul 10 '24 19:07 jay-schulz-openfellas

+1 with Nextcloud desktop client 3.13.2.

uselpa avatar Jul 11 '24 11:07 uselpa

This should really get attention as soon as possible. Having the same issue on Debian 12 with client 3.13.2 (AppImage) affecting several machines and messing permissions all over the place. I’m rolling back to 3.13.0 in the meantime.

casasfernando avatar Jul 12 '24 15:07 casasfernando

Same Problem on 3.13.2 with folder with local permissions like : drwxr-xr-x 164 FrancoisA Domain Users 20480 avril 15 17:33 'NIR 2023' dr-xr-xr-x 84 FrancoisA Domain Users 12288 juil. 12 17:16 'NIR 2024' dr-xr-xr-x 34 FrancoisA Domain Users 4096 juil. 5 17:41 NIR-Instances drwxr-xr-x 92 FrancoisA Domain Users 12288 juin 28 10:11 'NIR Permanentes'

Nextcloud can't create some new folder or file in "NIR 2024" or "NIR-Instances" If I change manually the perms, the sync works. But on the next sync with new folder, the folder perms are broken.

Nextcloud-desktop 3.13.2. Only on folders sync remote that I can't write.

webafrancois avatar Jul 16 '24 08:07 webafrancois

+1 ubuntu 22.04 and 24.04.

I've rolled back to 3.13.0

papamoose avatar Jul 26 '24 23:07 papamoose

I confirm the bug on Ubuntu 22.04 with Nextcloud client 3.13.2...

egourgoulhon avatar Aug 03 '24 16:08 egourgoulhon

Since the bug is currently not being addressed, is it advisable / what is the best way to downgrade to 3.13.0 on macOS? I have downloaded Nextcloud-3.13.0.pkg but simply installing that package does not work, so before doing it “my way ;-)” I was wondering if there is a generally recommended way?

uselpa avatar Aug 05 '24 14:08 uselpa

Since the bug is currently not being addressed, is it advisable / what is the best way to downgrade to 3.13.0 on macOS? I have downloaded Nextcloud-3.13.0.pkg but simply installing that package does not work, so before doing it “my way ;-)” I was wondering if there is a generally recommended way?

It is being addressed. On Mac OS you will have to remove the previous application installed, it won't downgrade. The installer for 3.13.0 will run but it won't do anything.

camilasan avatar Aug 07 '24 13:08 camilasan

I have the same problem and I reinforced it, publishing the problem again, but it seems that they are not correcting it, I am going to downgrade the application, because I synchronize my home, and there was a change in permissions. Does anyone know which version doesn't have this problem?

talesam avatar Aug 10 '24 17:08 talesam

anyone know which version doesn't have this problem?

3.12.3 works fine for me

shilin-da avatar Aug 10 '24 17:08 shilin-da

I have the same problem and I reinforced it, publishing the problem again, but it seems that they are not correcting it, I am going to downgrade the application, because I synchronize my home, and there was a change in permissions. Does anyone know which version doesn't have this problem?

3.13.0 is the last one that worked fine for me. 3.13.1 and 3.13.2 are broken.

casasfernando avatar Aug 10 '24 18:08 casasfernando

I have the same problem and I reinforced it, publishing the problem again, but it seems that they are not correcting it, I am going to downgrade the application, because I synchronize my home, and there was a change in permissions. Does anyone know which version doesn't have this problem?

3.13.0 is the last one that worked fine for me. 3.13.1 and 3.13.2 are broken.

I installed 3.13.0 and it's fine, I even set it to ignore updates. Now the question is, why are they ignoring this bug?😐

talesam avatar Aug 10 '24 18:08 talesam

I have the same problem and I reinforced it, publishing the problem again, but it seems that they are not correcting it, I am going to downgrade the application, because I synchronize my home, and there was a change in permissions. Does anyone know which version doesn't have this problem?

3.13.0 is the last one that worked fine for me. 3.13.1 and 3.13.2 are broken.

I installed 3.13.0 and it's fine, I even set it to ignore updates. Now the question is, why are they ignoring this bug?😐

talesam avatar Aug 10 '24 18:08 talesam

It is taking some time, for sure, but the PR(#6949) got a reviewer so it's certainly being noticed. Maybe keeping this thread alive could attract more eyes and get the problem resolved faster.

suiso67 avatar Aug 11 '24 02:08 suiso67

I tested using nextcloud 28.0.8.1 and 2x desktop-client 3.12.2

  • first fixed permissions: chmod u=rwX,g=rX,o= -R Nextcloud
  • ls -ld Nextcloud/folder/ -> drwxr-x---
  • touch Nextcloud/folder/test.txt
  • file gets synced to other client
  • remove file on other client
  • first client: ls -ld Nextcloud/folder/ -> drwxr-x-w-

Note: only write permission is set for others

Second scenario (only one client):

  • chmod u=rwX,g=rX,o= -R Nextcloud
  • mkdir Nextcloud/existing_folder/new_folder
  • touch Nextcloud/existing_folder/new_folder/file.txt after sync:
  • ls -ld Nextcloud/existing_folder/new_folder/file.txt -> drwxrwxr--
  • ls -ld Nextcloud/existing_folder/new_folder -> drwxrwxrwx
  • ls -ld Nextcloud/existing_folder -> drwxrwx-w-
  • ls -ld Nextcloud -> drwxr-x---

Note: existing folders only get write permission for others, new folders get rwx for others.

Same behavior in group folders.

bbx-github avatar Aug 13 '24 23:08 bbx-github

If Nextcloud really needs to make a file writable, then the umask setting should be used.

E.g. if umask is 0022, the Nextcloud must not set a file writable to group and would.

bjoernv avatar Aug 21 '24 21:08 bjoernv

@ltoenning wrote:

Maybe this is caused by the the following change:

https://github.com/nextcloud/desktop/pull/6853/files#diff-a0c50fd8b42d2ffff94466bbbf73b51fcd4aa01e607c278682a53e520bcf8c96L460-R459

where writePerms equals to "owner", "group" and "others" write permssion?

During debugging I found another place, where permissions are changed to user, group and would writable:

src/common/filesystembase.cpp:

void FileSystem::setFileReadOnly(const QString &filename, bool readonly)
{
    QFile file(filename);
    QFile::Permissions permissions = file.permissions();

    QFile::Permissions allWritePermissions =
        QFile::WriteUser | QFile::WriteGroup | QFile::WriteOther | QFile::WriteOwner;
    static QFile::Permissions defaultWritePermissions = getDefaultWritePermissions();

    permissions &= ~allWritePermissions;
    if (!readonly) {
        permissions |= defaultWritePermissions;
    }
    file.setPermissions(permissions);
}

bjoernv avatar Aug 21 '24 21:08 bjoernv

I don't understand how they are ignoring something serious like this. Messing with file permissions, making everything executable is regrettable and serious, even more so for me who uses $HOME to synchronize, I'm using the old version that doesn't mess up and I got stuck on it.

talesam avatar Aug 22 '24 00:08 talesam

I don't understand how they are ignoring something serious like this. Messing with file permissions, making everything executable is regrettable and serious, even more so for me who uses $HOME to synchronize, I'm using the old version that doesn't mess up and I got stuck on it.

This should be a security incident IMO.

papamoose avatar Aug 22 '24 01:08 papamoose

How is this security issue open for more than seven weeks now without a fix?

Complete lack of security culture.

mdierksen avatar Aug 22 '24 11:08 mdierksen

Today's 3.13.3 doesn't seem to fix it (on macOS at least).

uselpa avatar Aug 26 '24 14:08 uselpa

It would be awesome if Nextcloud started caring about file permissions in general.

7 years later after I reported https://github.com/nextcloud/server/issues/6725, I am still running complicated hack scripts periodically, as file permissions are out of sync across machines, by design.

C0rn3j avatar Aug 30 '24 11:08 C0rn3j

Has anyone started the process of getting a CVE for this?

I've requested one, but haven't heard anything about it.

bootlesshacker avatar Sep 03 '24 16:09 bootlesshacker

I looked into this issue and the culprit seems to be commit 82197c7a82629c745a9b6ded4b32c5189abbb6d6.

Before this commit setFolderPermissions used to only set owner_write on the second evaluation of the passed in permissions. This commit changed the behaviour to set writePerms for owner, group and other. And as the options are options add. The folder will be world writable.

I suggest reversing that change with the applied patch.

permissions.PATCH

sroas avatar Sep 09 '24 11:09 sroas

This should be a security incident IMO.

That sounds like the correct way.

As per our security policy security and privacy related issues and incidents should be reported using our HackerOne Program which is monitored even outside of office hours. That's much more helpful than a report under almost 1k issues.

I did that now and escalated it internally directly to the respective team. It should be picked up soon and an official comment should be coming afterwards.

nickvergessen avatar Sep 09 '24 13:09 nickvergessen