Skip to content

fix(UI): validate E2EE file names #10238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Rello wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix(UI): validate E2EE file names #10238

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
68c082f
fix(UI): Make FolderMetadata enforce that every decrypted E2EE filena…
Rello Jun 24, 2026
91d3ec0
Merge branch 'master' into bugfix/E2eeFolder
Rello Jun 24, 2026
047b9ec
fix(UI): test fixes
Rello Jun 24, 2026
09e3864
Remove file deletion on metadata error
Rello Jun 25, 2026
b05f17b
Add checks for '.' and '..' in filename validation
Rello Jun 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 70 additions & 9 deletions src/libsync/foldermetadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
/*
* SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: GPL-2.0-or-later
Expand All @@ -9,6 +9,7 @@
#include "clientsideencryption.h"
#include "clientsideencryptionjobs.h"
#include <common/checksums.h>
#include <QDir>
#include <QJsonArray>
#include <QJsonDocument>
#include <QSslCertificate>
Expand Down Expand Up @@ -49,6 +50,22 @@
}
}

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");
Expand Down Expand Up @@ -119,6 +136,11 @@
_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;
Expand Down Expand Up @@ -254,12 +276,20 @@

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;
}
Expand Down Expand Up @@ -344,12 +374,19 @@

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();

Expand Down Expand Up @@ -503,10 +540,17 @@
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 {};
Comment on lines +554 to +557

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fail metadata setup when an encrypted name is unsafe

When an existing encrypted folder's metadata contains an invalid original name, this branch returns an empty EncryptedFile; setupExistingMetadata() then just skips appending it and still marks the metadata valid. In that scenario the corresponding server entry remains in the PROPFIND results without an e2eMangledName/encrypted flag, so discovery can treat the encrypted blob itself as a normal plaintext child under its mangled server name instead of failing the sync. Please make parsing fail the metadata setup, or otherwise filter that remote entry, rather than silently dropping the mapping.

Useful? React with 👍 / 👎.

}

EncryptedFile file;
file.encryptedFilename = encryptedFilename;
Expand All @@ -516,7 +560,7 @@
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();

Expand All @@ -530,6 +574,11 @@

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);
Expand Down Expand Up @@ -723,6 +772,12 @@

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);
Expand Down Expand Up @@ -921,6 +976,12 @@
return;
}

if (!isOriginalFilenameValid(f.originalFilename)) {
qCWarning(lcCseMetadata()) << "Could not add encrypted file with invalid original file name.";
_account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError);
return;

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Propagate invalid-name rejection back to uploads

When a POSIX client syncs an E2EE file whose basename contains a backslash, local discovery can still allow it unless Windows compatibility or server-forbidden characters are active, but this new guard only returns from addEncryptedFile(). PropagateUploadEncrypted::slotFolderEncryptedMetadataReceived() does not observe that failure and continues with the generated encrypted filename and upload, so the server can receive an encrypted blob that is missing from the folder metadata and cannot be mapped/decrypted in later syncs. Please make this rejection visible to the caller and abort the upload instead of silently dropping the metadata entry.

Useful? React with 👍 / 👎.

}

for (int i = 0; i < _files.size(); ++i) {
if (_files.at(i).originalFilename == f.originalFilename) {
_files.removeAt(i);
Expand Down
2 changes: 2 additions & 0 deletions src/libsync/foldermetadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
57 changes: 57 additions & 0 deletions test/testclientsideencryptionv2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,63 @@ private slots:
QVERIFY(!metadataFromJson->isValid());
}

void testOriginalFilenameValidation_data()
{
QTest::addColumn<QString>("originalFilename");
QTest::addColumn<bool>("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<FolderMetadata> 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"
Expand Down
Loading