From 3135a1f54dea82571442feceff16b16e54213017 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 ee7f7ae3a847a..c61edc96e34eb 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1066,6 +1066,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; @@ -1093,6 +1097,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 4cb604b9aa7da..0c44ba4b472b3 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 ff38d739118ae..7d7344b3a41d8 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -103,6 +103,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 a4619535561b1..0d02a623adcab 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -340,6 +340,8 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem bool _isAnyInvalidCharChild = false; bool _isAnyCaseClashChild = false; + bool isPermissionsInvalid = false; + QString _discoveryResult; };