From 68c082fe76064fe7ac3f19373f7d4dbfa4d7f2a7 Mon Sep 17 00:00:00 2001 From: Rello Date: Wed, 24 Jun 2026 12:04:11 +0200 Subject: [PATCH 1/4] fix(UI): Make FolderMetadata enforce that every decrypted E2EE filename is a single path segment before it can enter discovery. Signed-off-by: Rello --- src/libsync/foldermetadata.cpp | 79 +++++++++++++++++++++++++---- src/libsync/foldermetadata.h | 2 + test/testclientsideencryptionv2.cpp | 57 +++++++++++++++++++++ 3 files changed, 129 insertions(+), 9 deletions(-) diff --git a/src/libsync/foldermetadata.cpp b/src/libsync/foldermetadata.cpp index d628eff04777d..2c7f0bf124d78 100644 --- a/src/libsync/foldermetadata.cpp +++ b/src/libsync/foldermetadata.cpp @@ -9,6 +9,7 @@ #include "clientsideencryption.h" #include "clientsideencryptionjobs.h" #include +#include #include #include #include @@ -49,6 +50,22 @@ QString metadataStringFromOCsDocument(const QJsonDocument &ocsDoc) } } +bool FolderMetadata::isOriginalFilenameValid(const QString &originalFilename) +{ + if (originalFilename.isEmpty()) { + return false; + } + + if (originalFilename.contains(QLatin1Char('/')) + || originalFilename.contains(QLatin1Char('\\')) + || originalFilename.contains(QChar(0))) { + return false; + } + + const auto slashPrefixedName = QStringLiteral("/") + originalFilename; + return QDir::cleanPath(slashPrefixedName) == slashPrefixedName; +} + bool FolderMetadata::EncryptedFile::isDirectory() const { return mimetype.isEmpty() || mimetype == QByteArrayLiteral("inode/directory") || mimetype == QByteArrayLiteral("httpd/unix-directory"); @@ -119,6 +136,11 @@ void FolderMetadata::setupExistingMetadata(const QByteArray &metadata) _account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); return; } + if (_existingMetadataVersion < MetadataVersion::Version2_0 && !_initialSignature.isEmpty()) { + qCWarning(lcCseMetadata()) << "Could not setup legacy metadata with a V2 signature."; + _account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); + return; + } if (_existingMetadataVersion < MetadataVersion::Version2_0) { setupExistingMetadataLegacy(metadata); return; @@ -254,12 +276,20 @@ void FolderMetadata::setupExistingMetadata(const QByteArray &metadata) for (auto it = folders.constBegin(); it != folders.constEnd(); ++it) { const auto folderName = it.value().toString(); - if (!folderName.isEmpty()) { - EncryptedFile file; - file.encryptedFilename = it.key(); - file.originalFilename = folderName; - _files.push_back(file); + if (folderName.isEmpty()) { + continue; + } + + if (!isOriginalFilenameValid(folderName)) { + qCWarning(lcCseMetadata()) << "skipping encrypted folder" << it.key() << "metadata has an invalid file name"; + _account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); + continue; } + + EncryptedFile file; + file.encryptedFilename = it.key(); + file.originalFilename = folderName; + _files.push_back(file); } _isMetadataValid = true; } @@ -344,12 +374,19 @@ void FolderMetadata::setupExistingMetadataLegacy(const QByteArray &metadata) const auto decryptedFileObj = decryptedFileDoc.object(); - if (decryptedFileObj["filename"].toString().isEmpty()) { + const auto originalFilename = decryptedFileObj["filename"].toString(); + if (originalFilename.isEmpty()) { qCWarning(lcCseMetadata) << "decrypted metadata" << decryptedFileDoc.toJson(QJsonDocument::Compact) << "skipping encrypted file" << file.encryptedFilename << "metadata has an empty file name"; continue; } - file.originalFilename = decryptedFileObj["filename"].toString(); + if (!isOriginalFilenameValid(originalFilename)) { + qCWarning(lcCseMetadata) << "skipping encrypted file" << file.encryptedFilename << "metadata has an invalid file name"; + _account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); + continue; + } + + file.originalFilename = originalFilename; file.encryptionKey = QByteArray::fromBase64(decryptedFileObj["key"].toString().toLocal8Bit()); file.mimetype = decryptedFileObj["mimetype"].toString().toLocal8Bit(); @@ -503,10 +540,17 @@ bool FolderMetadata::isValid() const FolderMetadata::EncryptedFile FolderMetadata::parseEncryptedFileFromJson(const QString &encryptedFilename, const QJsonValue &fileJSON) const { const auto fileObj = fileJSON.toObject(); - if (fileObj["filename"].toString().isEmpty()) { + const auto originalFilename = fileObj["filename"].toString(); + if (originalFilename.isEmpty()) { qCWarning(lcCseMetadata()) << "skipping encrypted file" << encryptedFilename << "metadata has an empty file name"; return {}; } + + if (!isOriginalFilenameValid(originalFilename)) { + qCWarning(lcCseMetadata()) << "skipping encrypted file" << encryptedFilename << "metadata has an invalid file name"; + _account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); + return {}; + } EncryptedFile file; file.encryptedFilename = encryptedFilename; @@ -516,7 +560,7 @@ FolderMetadata::EncryptedFile FolderMetadata::parseEncryptedFileFromJson(const Q nonce = QByteArray::fromBase64(fileObj[nonceKey].toString().toLocal8Bit()); } file.initializationVector = nonce; - file.originalFilename = fileObj["filename"].toString(); + file.originalFilename = originalFilename; file.encryptionKey = QByteArray::fromBase64(fileObj["key"].toString().toLocal8Bit()); file.mimetype = fileObj["mimetype"].toString().toLocal8Bit(); @@ -530,6 +574,11 @@ FolderMetadata::EncryptedFile FolderMetadata::parseEncryptedFileFromJson(const Q QJsonObject FolderMetadata::convertFileToJsonObject(const EncryptedFile *encryptedFile) const { + if (!encryptedFile || !isOriginalFilenameValid(encryptedFile->originalFilename)) { + qCWarning(lcCseMetadata()) << "Metadata generation failed. Invalid original file name."; + return {}; + } + QJsonObject file; file.insert("key", QString(encryptedFile->encryptionKey.toBase64())); file.insert("filename", encryptedFile->originalFilename); @@ -723,6 +772,12 @@ QByteArray FolderMetadata::encryptedMetadataLegacy() QJsonObject files; for (auto it = _files.constBegin(), end = _files.constEnd(); it != end; ++it) { + if (!isOriginalFilenameValid(it->originalFilename)) { + qCWarning(lcCseMetadata) << "Metadata generation failed. Invalid original file name for encrypted file" << it->encryptedFilename; + _account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); + return {}; + } + QJsonObject encrypted; encrypted.insert("key", QString(it->encryptionKey.toBase64())); encrypted.insert("filename", it->originalFilename); @@ -921,6 +976,12 @@ void FolderMetadata::addEncryptedFile(const EncryptedFile &f) { return; } + if (!isOriginalFilenameValid(f.originalFilename)) { + qCWarning(lcCseMetadata()) << "Could not add encrypted file with invalid original file name."; + _account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); + return; + } + for (int i = 0; i < _files.size(); ++i) { if (_files.at(i).originalFilename == f.originalFilename) { _files.removeAt(i); diff --git a/src/libsync/foldermetadata.h b/src/libsync/foldermetadata.h index c1b126ac21342..aa53d42dc5cca 100644 --- a/src/libsync/foldermetadata.h +++ b/src/libsync/foldermetadata.h @@ -171,6 +171,8 @@ public slots: [[nodiscard]] QJsonObject convertFileToJsonObject(const EncryptedFile *encryptedFile) const; + [[nodiscard]] static bool isOriginalFilenameValid(const QString &originalFilename); + [[nodiscard]] MetadataVersion latestSupportedMetadataVersion() const; [[nodiscard]] bool parseFileDropPart(const QJsonDocument &doc); diff --git a/test/testclientsideencryptionv2.cpp b/test/testclientsideencryptionv2.cpp index 9e75af70e3ae7..0063effe4e97e 100644 --- a/test/testclientsideencryptionv2.cpp +++ b/test/testclientsideencryptionv2.cpp @@ -219,6 +219,63 @@ private slots: QVERIFY(!metadataFromJson->isValid()); } + void testOriginalFilenameValidation_data() + { + QTest::addColumn("originalFilename"); + QTest::addColumn("isValid"); + + QTest::newRow("plain file") << QStringLiteral("document.txt") << true; + QTest::newRow("plain folder") << QStringLiteral("Documents") << true; + QTest::newRow("hidden file") << QStringLiteral(".hidden") << true; + QTest::newRow("empty") << QString() << false; + QTest::newRow("current directory") << QStringLiteral(".") << false; + QTest::newRow("parent directory") << QStringLiteral("..") << false; + QTest::newRow("relative traversal") << QStringLiteral("../../poc_dir") << false; + QTest::newRow("embedded slash") << QStringLiteral("folder/file.txt") << false; + QTest::newRow("absolute path") << QStringLiteral("/tmp/poc") << false; + QTest::newRow("backslash") << QStringLiteral("folder\\file.txt") << false; + QTest::newRow("null byte") << QStringLiteral("file") + QChar(0) + QStringLiteral("name") << false; + } + + void testOriginalFilenameValidation() + { + QFETCH(QString, originalFilename); + QFETCH(bool, isValid); + + QCOMPARE(FolderMetadata::isOriginalFilenameValid(originalFilename), isValid); + } + + void testParseEncryptedFileFromJsonRejectsUnsafeOriginalFilename_data() + { + testOriginalFilenameValidation_data(); + } + + void testParseEncryptedFileFromJsonRejectsUnsafeOriginalFilename() + { + QFETCH(QString, originalFilename); + QFETCH(bool, isValid); + + QScopedPointer metadata(new FolderMetadata(_account, "/", FolderMetadata::FolderType::Root)); + QSignalSpy metadataSetupCompleteSpy(metadata.data(), &FolderMetadata::setupComplete); + metadataSetupCompleteSpy.wait(); + QCOMPARE(metadataSetupCompleteSpy.count(), 1); + QVERIFY(metadata->isValid()); + + const auto fileJson = QJsonObject{ + {QStringLiteral("filename"), originalFilename}, + {QStringLiteral("key"), QString::fromUtf8(QByteArrayLiteral("key").toBase64())}, + {QStringLiteral("mimetype"), QStringLiteral("application/octet-stream")}, + {QStringLiteral("nonce"), QString::fromUtf8(QByteArrayLiteral("nonce").toBase64())}, + {QStringLiteral("authenticationTag"), QString::fromUtf8(QByteArrayLiteral("tag").toBase64())}, + }; + + const auto parsedEncryptedFile = metadata->parseEncryptedFileFromJson(QStringLiteral("encrypted-name"), fileJson); + QCOMPARE(!parsedEncryptedFile.originalFilename.isEmpty(), isValid); + if (isValid) { + QCOMPARE(parsedEncryptedFile.originalFilename, originalFilename); + } + } + void testE2EeFolderMetadataSharing() { // instantiate empty metadata, add a file, and share with a second user "sharee" From 047b9ec9e3591aa047fb439db127dc7963686074 Mon Sep 17 00:00:00 2001 From: Rello Date: Wed, 24 Jun 2026 13:30:05 +0200 Subject: [PATCH 2/4] fix(UI): test fixes Signed-off-by: Rello --- src/libsync/foldermetadata.cpp | 11 +++++++---- src/libsync/foldermetadata.h | 2 +- src/libsync/propagateuploadencrypted.cpp | 9 ++++++++- test/testclientsideencryptionv2.cpp | 25 +++++++++++++++++++++--- test/testsecurefiledrop.cpp | 2 +- 5 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/libsync/foldermetadata.cpp b/src/libsync/foldermetadata.cpp index 2c7f0bf124d78..f6b288399f895 100644 --- a/src/libsync/foldermetadata.cpp +++ b/src/libsync/foldermetadata.cpp @@ -969,17 +969,17 @@ QByteArray FolderMetadata::prepareMetadataForSignature(const QJsonDocument &full return metdataModified.toJson(QJsonDocument::Compact); } -void FolderMetadata::addEncryptedFile(const EncryptedFile &f) { +bool FolderMetadata::addEncryptedFile(const EncryptedFile &f) { Q_ASSERT(_isMetadataValid); if (!_isMetadataValid) { qCWarning(lcCseMetadata()) << "Could not add encrypted file to non-initialized metadata!"; - return; + return false; } if (!isOriginalFilenameValid(f.originalFilename)) { qCWarning(lcCseMetadata()) << "Could not add encrypted file with invalid original file name."; _account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); - return; + return false; } for (int i = 0; i < _files.size(); ++i) { @@ -990,6 +990,7 @@ void FolderMetadata::addEncryptedFile(const EncryptedFile &f) { } _files.append(f); + return true; } const QByteArray FolderMetadata::binaryMetadataKeyForDecryption() const @@ -1062,7 +1063,9 @@ bool FolderMetadata::moveFromFileDropToFiles() _account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError); return false; } - addEncryptedFile(parsedEncryptedFile); + if (!addEncryptedFile(parsedEncryptedFile)) { + return false; + } } _fileDropEntries.clear(); diff --git a/src/libsync/foldermetadata.h b/src/libsync/foldermetadata.h index aa53d42dc5cca..a127e7abc79a9 100644 --- a/src/libsync/foldermetadata.h +++ b/src/libsync/foldermetadata.h @@ -146,7 +146,7 @@ class OWNCLOUDSYNC_EXPORT FolderMetadata : public QObject static MetadataVersion setupVersionFromExistingMetadata(const QByteArray &metadata); public slots: - void addEncryptedFile(const OCC::FolderMetadata::EncryptedFile &f); + [[nodiscard]] bool addEncryptedFile(const OCC::FolderMetadata::EncryptedFile &f); void removeEncryptedFile(const OCC::FolderMetadata::EncryptedFile &f); void removeAllEncryptedFiles(); diff --git a/src/libsync/propagateuploadencrypted.cpp b/src/libsync/propagateuploadencrypted.cpp index 212abba35ee98..f43b960417dad 100644 --- a/src/libsync/propagateuploadencrypted.cpp +++ b/src/libsync/propagateuploadencrypted.cpp @@ -163,7 +163,14 @@ void PropagateUploadEncrypted::slotFetchMetadataJobFinished(int statusCode, cons qCDebug(lcPropagateUploadEncrypted) << "Creating the metadata for the encrypted file."; - metadata->addEncryptedFile(encryptedFile); + if (!metadata->addEncryptedFile(encryptedFile)) { + qCWarning(lcPropagateUploadEncrypted()) << "There was an error encrypting the file, aborting upload. Invalid metadata file name."; + if (!info.isDir()) { + QFile::remove(_completeFileName); + } + emit error(); + return; + } qCDebug(lcPropagateUploadEncrypted) << "Metadata created, sending to the server."; diff --git a/test/testclientsideencryptionv2.cpp b/test/testclientsideencryptionv2.cpp index 0063effe4e97e..970740ca153b6 100644 --- a/test/testclientsideencryptionv2.cpp +++ b/test/testclientsideencryptionv2.cpp @@ -104,7 +104,7 @@ private slots: encryptedFile.originalFilename = fakeFileName; encryptedFile.mimetype = "application/octet-stream"; encryptedFile.initializationVector = EncryptionHelper::generateRandom(16); - metadata->addEncryptedFile(encryptedFile); + QVERIFY(metadata->addEncryptedFile(encryptedFile)); const auto encryptedMetadata = metadata->encryptedMetadata(); QVERIFY(!encryptedMetadata.isEmpty()); @@ -276,6 +276,25 @@ private slots: } } + void testAddEncryptedFileRejectsUnsafeOriginalFilename() + { + QScopedPointer metadata(new FolderMetadata(_account, "/", FolderMetadata::FolderType::Root)); + QSignalSpy metadataSetupCompleteSpy(metadata.data(), &FolderMetadata::setupComplete); + metadataSetupCompleteSpy.wait(); + QCOMPARE(metadataSetupCompleteSpy.count(), 1); + QVERIFY(metadata->isValid()); + + FolderMetadata::EncryptedFile encryptedFile; + encryptedFile.encryptionKey = EncryptionHelper::generateRandom(16); + encryptedFile.encryptedFilename = EncryptionHelper::generateRandomFilename(); + encryptedFile.originalFilename = QStringLiteral("folder\\file.txt"); + encryptedFile.mimetype = "application/octet-stream"; + encryptedFile.initializationVector = EncryptionHelper::generateRandom(16); + + QVERIFY(!metadata->addEncryptedFile(encryptedFile)); + QVERIFY(metadata->files().isEmpty()); + } + void testE2EeFolderMetadataSharing() { // instantiate empty metadata, add a file, and share with a second user "sharee" @@ -293,7 +312,7 @@ private slots: encryptedFile.originalFilename = fakeFileName; encryptedFile.mimetype = "application/octet-stream"; encryptedFile.initializationVector = EncryptionHelper::generateRandom(16); - metadata->addEncryptedFile(encryptedFile); + QVERIFY(metadata->addEncryptedFile(encryptedFile)); QVERIFY(metadata->addUser(_secondAccount->davUser(), _secondAccount->e2e()->getCertificate(), FolderMetadata::CertificateType::SoftwareNextcloudCertificate)); @@ -384,7 +403,7 @@ private slots: encryptedFile.originalFilename = fakeFileNameFromSecondUser; encryptedFile.mimetype = "application/octet-stream"; encryptedFile.initializationVector = EncryptionHelper::generateRandom(16); - metadataFromJsonForSecondUser->addEncryptedFile(encryptedFile); + QVERIFY(metadataFromJsonForSecondUser->addEncryptedFile(encryptedFile)); auto encryptedMetadataFromSecondUser = metadataFromJsonForSecondUser->encryptedMetadata(); encryptedMetadataFromSecondUser.replace("\"", "\\\""); diff --git a/test/testsecurefiledrop.cpp b/test/testsecurefiledrop.cpp index 8e7162ae34d7d..5ffc852c863f3 100644 --- a/test/testsecurefiledrop.cpp +++ b/test/testsecurefiledrop.cpp @@ -87,7 +87,7 @@ private slots: encryptedFile.originalFilename = fakeFileName; encryptedFile.mimetype = "application/octet-stream"; encryptedFile.initializationVector = EncryptionHelper::generateRandom(16); - metadata->addEncryptedFile(encryptedFile); + QVERIFY(metadata->addEncryptedFile(encryptedFile)); } QJsonObject fakeFileDropPart; From 09e3864fab834a7f9481e85f4e85a702b81e2634 Mon Sep 17 00:00:00 2001 From: Rello Date: Thu, 25 Jun 2026 13:44:59 +0200 Subject: [PATCH 3/4] Remove file deletion on metadata error Removed file deletion logic for non-directory uploads. Signed-off-by: Rello --- src/libsync/propagateuploadencrypted.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libsync/propagateuploadencrypted.cpp b/src/libsync/propagateuploadencrypted.cpp index f43b960417dad..11ff3472f0129 100644 --- a/src/libsync/propagateuploadencrypted.cpp +++ b/src/libsync/propagateuploadencrypted.cpp @@ -165,9 +165,6 @@ void PropagateUploadEncrypted::slotFetchMetadataJobFinished(int statusCode, cons if (!metadata->addEncryptedFile(encryptedFile)) { qCWarning(lcPropagateUploadEncrypted()) << "There was an error encrypting the file, aborting upload. Invalid metadata file name."; - if (!info.isDir()) { - QFile::remove(_completeFileName); - } emit error(); return; } From b05f17b43a499f895691de468092b14dc5ae95b8 Mon Sep 17 00:00:00 2001 From: Rello Date: Thu, 25 Jun 2026 15:13:11 +0200 Subject: [PATCH 4/4] Add checks for '.' and '..' in filename validation Signed-off-by: Rello --- src/libsync/foldermetadata.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libsync/foldermetadata.cpp b/src/libsync/foldermetadata.cpp index f6b288399f895..8318e7f19561d 100644 --- a/src/libsync/foldermetadata.cpp +++ b/src/libsync/foldermetadata.cpp @@ -56,6 +56,11 @@ bool FolderMetadata::isOriginalFilenameValid(const QString &originalFilename) return false; } + if (originalFilename == QStringLiteral(".") + || originalFilename == QStringLiteral("..")) { + return false; + } + if (originalFilename.contains(QLatin1Char('/')) || originalFilename.contains(QLatin1Char('\\')) || originalFilename.contains(QChar(0))) {