From 02029e892f83dd7715fc6541fdda588aa538b3a9 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 5c8fbc097..9da7497f7 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 c5e22abb3..ec47ab3c7 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 05bdf1554..7300a4c5f 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 3ca34e94f..6cd226f2e 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 2e801de34..bb932f568 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 2931a3d87..9dbdde6a7 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 193da94df..f10fc6135 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 ac48c0be2..0f8691cd0 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 d90348af4..46ee49621 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -343,6 +343,8 @@ public: bool _isLivePhoto = false; QString _livePhotoFile; + bool isPermissionsInvalid = false; + QString _discoveryResult; };