From 56c313646f49934cd9109b9ae78732b0b127a15a Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 20 Nov 2024 14:09:08 +0100 Subject: [PATCH] ensure no world writable permissions in Nextcloud sync folder past versions may have wrongly set the write permissions for all remove this under the hypothesis that newly created files or folders gets more restrictive permissions and that past files or folders should be updated to get the same permissions Signed-off-by: Matthieu Gallien --- src/csync/csync.h | 2 ++ src/csync/vio/csync_vio_local_unix.cpp | 2 ++ src/libsync/discovery.cpp | 11 +++++++++++ src/libsync/discoveryphase.cpp | 1 + src/libsync/discoveryphase.h | 1 + src/libsync/filesystem.cpp | 1 + src/libsync/owncloudpropagator.cpp | 14 +++----------- src/libsync/syncengine.cpp | 4 ++++ src/libsync/syncfileitem.h | 2 ++ 9 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/csync/csync.h b/src/csync/csync.h index 5c8fbc0978b6b..9da7497f75c7f 100644 --- a/src/csync/csync.h +++ b/src/csync/csync.h @@ -217,6 +217,7 @@ struct OCSYNC_EXPORT csync_file_stat_s { bool is_hidden BITFIELD(1); // Not saved in the DB, only used during discovery for local files. bool isE2eEncrypted BITFIELD(1); bool is_metadata_missing BITFIELD(1); // Indicates the file has missing metadata, f.ex. the file is not a placeholder in case of vfs. + bool isPermissionsInvalid BITFIELD(1); QByteArray path; QByteArray rename_path; @@ -244,6 +245,7 @@ struct OCSYNC_EXPORT csync_file_stat_s { , is_hidden(false) , isE2eEncrypted(false) , is_metadata_missing(false) + , isPermissionsInvalid(false) { } }; diff --git a/src/csync/vio/csync_vio_local_unix.cpp b/src/csync/vio/csync_vio_local_unix.cpp index c5e22abb3c8b0..ec47ab3c7c994 100644 --- a/src/csync/vio/csync_vio_local_unix.cpp +++ b/src/csync/vio/csync_vio_local_unix.cpp @@ -170,5 +170,7 @@ static int _csync_vio_local_stat_mb(const mbchar_t *wuri, csync_file_stat_t *buf buf->inode = sb.st_ino; buf->modtime = sb.st_mtime; buf->size = sb.st_size; + buf->isPermissionsInvalid = (sb.st_mode & S_IWOTH) == S_IWOTH; + return 0; } diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 05bdf1554bc0b..7300a4c5f9a1e 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1070,6 +1070,10 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( if (_queryLocal != NormalQuery && _queryServer != NormalQuery) recurse = false; + if (localEntry.isPermissionsInvalid) { + recurse = true; + } + if ((item->_direction == SyncFileItem::Down || item->_instruction == CSYNC_INSTRUCTION_CONFLICT || item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC) && (item->_modtime <= 0 || item->_modtime >= 0xFFFFFFFF)) { item->_instruction = CSYNC_INSTRUCTION_ERROR; @@ -1097,6 +1101,13 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( } } + if (localEntry.isPermissionsInvalid && item->_instruction == CSyncEnums::CSYNC_INSTRUCTION_NONE) { + item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; + item->_direction = SyncFileItem::Down; + } + + item->isPermissionsInvalid = localEntry.isPermissionsInvalid; + auto recurseQueryLocal = _queryLocal == ParentNotChanged ? ParentNotChanged : localEntry.isDirectory || item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist; processFileFinalize(item, path, recurse, recurseQueryLocal, recurseQueryServer); }; diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 3ca34e94f50fb..6cd226f2ee1a5 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -348,6 +348,7 @@ void DiscoverySingleLocalDirectoryJob::run() { i.isSymLink = dirent->type == ItemTypeSoftLink; i.isVirtualFile = dirent->type == ItemTypeVirtualFile || dirent->type == ItemTypeVirtualFileDownload; i.isMetadataMissing = dirent->is_metadata_missing; + i.isPermissionsInvalid = dirent->isPermissionsInvalid; i.type = dirent->type; results.push_back(i); } diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index 2e801de34cd53..bb932f568b4db 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -106,6 +106,7 @@ struct LocalInfo bool isVirtualFile = false; bool isSymLink = false; bool isMetadataMissing = false; + bool isPermissionsInvalid = false; [[nodiscard]] bool isValid() const { return !name.isNull(); } }; diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 2931a3d877c7e..9dbdde6a7474c 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -468,6 +468,7 @@ bool FileSystem::setFolderPermissions(const QString &path, case OCC::FileSystem::FolderPermissions::ReadOnly: break; case OCC::FileSystem::FolderPermissions::ReadWrite: + std::filesystem::permissions(stdStrPath, std::filesystem::perms::others_write, std::filesystem::perm_options::remove); std::filesystem::permissions(stdStrPath, std::filesystem::perms::owner_write, std::filesystem::perm_options::add); break; } diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 193da94df3e9a..f10fc6135913c 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -1461,15 +1461,9 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status) try { if (FileSystem::fileExists(propagator()->fullLocalPath(_item->_file))) { FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_file), FileSystem::FolderPermissions::ReadOnly); - qCDebug(lcDirectory) << "old permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions()); - std::filesystem::permissions(propagator()->fullLocalPath(_item->_file).toStdWString(), std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write, std::filesystem::perm_options::remove); - qCDebug(lcDirectory) << "new permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions()); } if (!_item->_renameTarget.isEmpty() && FileSystem::fileExists(propagator()->fullLocalPath(_item->_renameTarget))) { FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_renameTarget), FileSystem::FolderPermissions::ReadOnly); - qCDebug(lcDirectory) << "old permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions()); - std::filesystem::permissions(propagator()->fullLocalPath(_item->_renameTarget).toStdWString(), std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write, std::filesystem::perm_options::remove); - qCDebug(lcDirectory) << "new permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions()); } } catch (const std::filesystem::filesystem_error &e) @@ -1481,15 +1475,13 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status) } else { try { if (FileSystem::fileExists(propagator()->fullLocalPath(_item->_file))) { + qCDebug(lcDirectory) << propagator()->fullLocalPath(_item->_file) << "old permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions()); FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_file), FileSystem::FolderPermissions::ReadWrite); - qCDebug(lcDirectory) << "old permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions()); - std::filesystem::permissions(propagator()->fullLocalPath(_item->_file).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add); - qCDebug(lcDirectory) << "new permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions()); + qCDebug(lcDirectory) << propagator()->fullLocalPath(_item->_file) << "new permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions()); } if (!_item->_renameTarget.isEmpty() && FileSystem::fileExists(propagator()->fullLocalPath(_item->_renameTarget))) { - FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_renameTarget), FileSystem::FolderPermissions::ReadWrite); qCDebug(lcDirectory) << "old permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions()); - std::filesystem::permissions(propagator()->fullLocalPath(_item->_renameTarget).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add); + FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_renameTarget), FileSystem::FolderPermissions::ReadWrite); qCDebug(lcDirectory) << "new permissions" << static_cast(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions()); } } diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index ac48c0be2c2cf..0f8691cd05960 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -363,6 +363,10 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item) const bool isReadOnly = !item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite); modificationHappened = FileSystem::setFileReadOnlyWeak(filePath, isReadOnly); } + if (item->isPermissionsInvalid) { + const auto isReadOnly = !item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite); + FileSystem::setFileReadOnly(filePath, isReadOnly); + } modificationHappened |= item->_size != prev._fileSize; diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index d90348af4ebda..46ee49621c688 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -343,6 +343,8 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem bool _isLivePhoto = false; QString _livePhotoFile; + bool isPermissionsInvalid = false; + QString _discoveryResult; };