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
+169 −18
Open

fix(UI): validate E2EE file names #10238

Show file tree
Hide file tree
Changes from 3 commits
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
88 changes: 76 additions & 12 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 @@ -914,11 +969,17 @@
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) {
Expand All @@ -929,6 +990,7 @@
}

_files.append(f);
return true;
}

const QByteArray FolderMetadata::binaryMetadataKeyForDecryption() const
Expand Down Expand Up @@ -1001,7 +1063,9 @@
_account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError);
return false;
}
addEncryptedFile(parsedEncryptedFile);
if (!addEncryptedFile(parsedEncryptedFile)) {
return false;
}
}

_fileDropEntries.clear();
Expand Down
4 changes: 3 additions & 1 deletion src/libsync/foldermetadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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
9 changes: 8 additions & 1 deletion src/libsync/propagateuploadencrypted.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Comment thread
Rello marked this conversation as resolved.
Outdated
emit error();

@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 Finish encrypted mkdir on invalid names

When this new rejection is reached for an encrypted directory upload, such as a POSIX folder named foo\bar inside an E2EE parent, the only consumer in PropagateRemoteMkdir::slotMkdir() connects PropagateUploadEncrypted::error to a logging-only lambda (src/libsync/propagateremotemkdir.cpp:212), unlike file uploads which call done(). Emitting error() here therefore leaves the mkdir job in _activeJobList with no MKCOL or completion signal, so the sync can stall instead of failing the item; make the mkdir path finish/cleanup on this failure.

Useful? React with 👍 / 👎.

return;
}

qCDebug(lcPropagateUploadEncrypted) << "Metadata created, sending to the server.";

Expand Down
82 changes: 79 additions & 3 deletions test/testclientsideencryptionv2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -219,6 +219,82 @@ 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 testAddEncryptedFileRejectsUnsafeOriginalFilename()
{
QScopedPointer<FolderMetadata> 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"
Expand All @@ -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));

Expand Down Expand Up @@ -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("\"", "\\\"");
Expand Down
2 changes: 1 addition & 1 deletion test/testsecurefiledrop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading