diff --git a/src/libsync/foldermetadata.cpp b/src/libsync/foldermetadata.cpp index d628eff04777d..8318e7f19561d 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,27 @@ QString metadataStringFromOCsDocument(const QJsonDocument &ocsDoc) } } +bool FolderMetadata::isOriginalFilenameValid(const QString &originalFilename) +{ + if (originalFilename.isEmpty()) { + return false; + } + + if (originalFilename == QStringLiteral(".") + || originalFilename == QStringLiteral("..")) { + 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 +141,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 +281,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 +379,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 +545,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 +565,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 +579,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 +777,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); @@ -914,11 +974,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 false; } for (int i = 0; i < _files.size(); ++i) { @@ -929,6 +995,7 @@ void FolderMetadata::addEncryptedFile(const EncryptedFile &f) { } _files.append(f); + return true; } const QByteArray FolderMetadata::binaryMetadataKeyForDecryption() const @@ -1001,7 +1068,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 c1b126ac21342..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(); @@ -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/src/libsync/propagateuploadencrypted.cpp b/src/libsync/propagateuploadencrypted.cpp index 212abba35ee98..11ff3472f0129 100644 --- a/src/libsync/propagateuploadencrypted.cpp +++ b/src/libsync/propagateuploadencrypted.cpp @@ -163,7 +163,11 @@ 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."; + emit error(); + return; + } qCDebug(lcPropagateUploadEncrypted) << "Metadata created, sending to the server."; diff --git a/test/testclientsideencryptionv2.cpp b/test/testclientsideencryptionv2.cpp index 9e75af70e3ae7..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()); @@ -219,6 +219,82 @@ 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 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" @@ -236,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)); @@ -327,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;