Skip to content

Commit 56c3136

Browse files
committed
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 <[email protected]>
1 parent 4349b4a commit 56c3136

File tree

9 files changed

+27
-11
lines changed

9 files changed

+27
-11
lines changed

src/csync/csync.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ struct OCSYNC_EXPORT csync_file_stat_s {
217217
bool is_hidden BITFIELD(1); // Not saved in the DB, only used during discovery for local files.
218218
bool isE2eEncrypted BITFIELD(1);
219219
bool is_metadata_missing BITFIELD(1); // Indicates the file has missing metadata, f.ex. the file is not a placeholder in case of vfs.
220+
bool isPermissionsInvalid BITFIELD(1);
220221

221222
QByteArray path;
222223
QByteArray rename_path;
@@ -244,6 +245,7 @@ struct OCSYNC_EXPORT csync_file_stat_s {
244245
, is_hidden(false)
245246
, isE2eEncrypted(false)
246247
, is_metadata_missing(false)
248+
, isPermissionsInvalid(false)
247249
{ }
248250
};
249251

src/csync/vio/csync_vio_local_unix.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,5 +170,7 @@ static int _csync_vio_local_stat_mb(const mbchar_t *wuri, csync_file_stat_t *buf
170170
buf->inode = sb.st_ino;
171171
buf->modtime = sb.st_mtime;
172172
buf->size = sb.st_size;
173+
buf->isPermissionsInvalid = (sb.st_mode & S_IWOTH) == S_IWOTH;
174+
173175
return 0;
174176
}

src/libsync/discovery.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,6 +1070,10 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
10701070
if (_queryLocal != NormalQuery && _queryServer != NormalQuery)
10711071
recurse = false;
10721072

1073+
if (localEntry.isPermissionsInvalid) {
1074+
recurse = true;
1075+
}
1076+
10731077
if ((item->_direction == SyncFileItem::Down || item->_instruction == CSYNC_INSTRUCTION_CONFLICT || item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC) &&
10741078
(item->_modtime <= 0 || item->_modtime >= 0xFFFFFFFF)) {
10751079
item->_instruction = CSYNC_INSTRUCTION_ERROR;
@@ -1097,6 +1101,13 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
10971101
}
10981102
}
10991103

1104+
if (localEntry.isPermissionsInvalid && item->_instruction == CSyncEnums::CSYNC_INSTRUCTION_NONE) {
1105+
item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
1106+
item->_direction = SyncFileItem::Down;
1107+
}
1108+
1109+
item->isPermissionsInvalid = localEntry.isPermissionsInvalid;
1110+
11001111
auto recurseQueryLocal = _queryLocal == ParentNotChanged ? ParentNotChanged : localEntry.isDirectory || item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist;
11011112
processFileFinalize(item, path, recurse, recurseQueryLocal, recurseQueryServer);
11021113
};

src/libsync/discoveryphase.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@ void DiscoverySingleLocalDirectoryJob::run() {
348348
i.isSymLink = dirent->type == ItemTypeSoftLink;
349349
i.isVirtualFile = dirent->type == ItemTypeVirtualFile || dirent->type == ItemTypeVirtualFileDownload;
350350
i.isMetadataMissing = dirent->is_metadata_missing;
351+
i.isPermissionsInvalid = dirent->isPermissionsInvalid;
351352
i.type = dirent->type;
352353
results.push_back(i);
353354
}

src/libsync/discoveryphase.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ struct LocalInfo
106106
bool isVirtualFile = false;
107107
bool isSymLink = false;
108108
bool isMetadataMissing = false;
109+
bool isPermissionsInvalid = false;
109110
[[nodiscard]] bool isValid() const { return !name.isNull(); }
110111
};
111112

src/libsync/filesystem.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,7 @@ bool FileSystem::setFolderPermissions(const QString &path,
468468
case OCC::FileSystem::FolderPermissions::ReadOnly:
469469
break;
470470
case OCC::FileSystem::FolderPermissions::ReadWrite:
471+
std::filesystem::permissions(stdStrPath, std::filesystem::perms::others_write, std::filesystem::perm_options::remove);
471472
std::filesystem::permissions(stdStrPath, std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
472473
break;
473474
}

src/libsync/owncloudpropagator.cpp

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,15 +1461,9 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
14611461
try {
14621462
if (FileSystem::fileExists(propagator()->fullLocalPath(_item->_file))) {
14631463
FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_file), FileSystem::FolderPermissions::ReadOnly);
1464-
qCDebug(lcDirectory) << "old permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions());
1465-
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);
1466-
qCDebug(lcDirectory) << "new permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions());
14671464
}
14681465
if (!_item->_renameTarget.isEmpty() && FileSystem::fileExists(propagator()->fullLocalPath(_item->_renameTarget))) {
14691466
FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_renameTarget), FileSystem::FolderPermissions::ReadOnly);
1470-
qCDebug(lcDirectory) << "old permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions());
1471-
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);
1472-
qCDebug(lcDirectory) << "new permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions());
14731467
}
14741468
}
14751469
catch (const std::filesystem::filesystem_error &e)
@@ -1481,15 +1475,13 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
14811475
} else {
14821476
try {
14831477
if (FileSystem::fileExists(propagator()->fullLocalPath(_item->_file))) {
1478+
qCDebug(lcDirectory) << propagator()->fullLocalPath(_item->_file) << "old permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions());
14841479
FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_file), FileSystem::FolderPermissions::ReadWrite);
1485-
qCDebug(lcDirectory) << "old permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions());
1486-
std::filesystem::permissions(propagator()->fullLocalPath(_item->_file).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
1487-
qCDebug(lcDirectory) << "new permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions());
1480+
qCDebug(lcDirectory) << propagator()->fullLocalPath(_item->_file) << "new permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_file).toStdWString()).permissions());
14881481
}
14891482
if (!_item->_renameTarget.isEmpty() && FileSystem::fileExists(propagator()->fullLocalPath(_item->_renameTarget))) {
1490-
FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_renameTarget), FileSystem::FolderPermissions::ReadWrite);
14911483
qCDebug(lcDirectory) << "old permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions());
1492-
std::filesystem::permissions(propagator()->fullLocalPath(_item->_renameTarget).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
1484+
FileSystem::setFolderPermissions(propagator()->fullLocalPath(_item->_renameTarget), FileSystem::FolderPermissions::ReadWrite);
14931485
qCDebug(lcDirectory) << "new permissions" << static_cast<int>(std::filesystem::status(propagator()->fullLocalPath(_item->_renameTarget).toStdWString()).permissions());
14941486
}
14951487
}

src/libsync/syncengine.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,10 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item)
363363
const bool isReadOnly = !item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite);
364364
modificationHappened = FileSystem::setFileReadOnlyWeak(filePath, isReadOnly);
365365
}
366+
if (item->isPermissionsInvalid) {
367+
const auto isReadOnly = !item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite);
368+
FileSystem::setFileReadOnly(filePath, isReadOnly);
369+
}
366370

367371
modificationHappened |= item->_size != prev._fileSize;
368372

src/libsync/syncfileitem.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,8 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem
343343
bool _isLivePhoto = false;
344344
QString _livePhotoFile;
345345

346+
bool isPermissionsInvalid = false;
347+
346348
QString _discoveryResult;
347349
};
348350

0 commit comments

Comments
 (0)