Merge pull request #5335 from nextcloud/backport/5323/stable-3.6

[stable-3.6] Bugfix/e2ee vulnerability empty metadatakeys
nextcloud · Jan 18, 2023 · ff1db46 · ff1db46
2 parents b7e2d19 + c2a4af5
commit ff1db46
Show file tree

Hide file tree

Showing 8 changed files with 86 additions and 48 deletions.
diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp
@@ -1390,6 +1390,12 @@ void FolderMetadata::setupExistingMetadata(const QByteArray& metadata)
   QJsonDocument metaDataDoc = QJsonDocument::fromJson(metaDataStr.toLocal8Bit());
   QJsonObject metadataObj = metaDataDoc.object()["metadata"].toObject();
   QJsonObject metadataKeys = metadataObj["metadataKeys"].toObject();
+
+  if (metadataKeys.isEmpty()) {
+      qCDebug(lcCse()) << "Could not setup existing metadata with missing metadataKeys!";
+      return;
+  }
+
   QByteArray sharing = metadataObj["sharing"].toString().toLocal8Bit();
   QJsonObject files = metaDataDoc.object()["files"].toObject();
 
@@ -1418,14 +1424,19 @@ void FolderMetadata::setupExistingMetadata(const QByteArray& metadata)
   // Cool, We actually have the key, we can decrypt the rest of the metadata.
   qCDebug(lcCse) << "Sharing: " << sharing;
   if (sharing.size()) {
-      auto sharingDecrypted = decryptJsonObject(sharing, _metadataKeys.last());
-      qCDebug(lcCse) << "Sharing Decrypted" << sharingDecrypted;
-
-      //Sharing is also a JSON object, so extract it and populate.
-      auto sharingDoc = QJsonDocument::fromJson(sharingDecrypted);
-      auto sharingObj = sharingDoc.object();
-      for (auto it = sharingObj.constBegin(), end = sharingObj.constEnd(); it != end; it++) {
-        _sharing.push_back({it.key(), it.value().toString()});
+      const auto metaDataKey = !_metadataKeys.isEmpty() ? _metadataKeys.last() : QByteArray{};
+      if (metaDataKey.isEmpty()) {
+          qCDebug(lcCse) << "Failed to decrypt sharing! Empty metadata key!";
+      } else {
+          auto sharingDecrypted = decryptJsonObject(sharing, metaDataKey);
+          qCDebug(lcCse) << "Sharing Decrypted" << sharingDecrypted;
+
+          // Sharing is also a JSON object, so extract it and populate.
+          auto sharingDoc = QJsonDocument::fromJson(sharingDecrypted);
+          auto sharingObj = sharingDoc.object();
+          for (auto it = sharingObj.constBegin(), end = sharingObj.constEnd(); it != end; it++) {
+              _sharing.push_back({it.key(), it.value().toString()});
+          }
       }
   } else {
       qCDebug(lcCse) << "Skipping sharing section since it is empty";
@@ -1441,9 +1452,9 @@ void FolderMetadata::setupExistingMetadata(const QByteArray& metadata)
         file.initializationVector = QByteArray::fromBase64(fileObj["initializationVector"].toString().toLocal8Bit());
 
         //Decrypt encrypted part
-        QByteArray key = _metadataKeys[file.metadataKey];
+        const auto key = _metadataKeys.value(file.metadataKey, {});
         auto encryptedFile = fileObj["encrypted"].toString().toLocal8Bit();
-        auto decryptedFile = decryptJsonObject(encryptedFile, key);
+        auto decryptedFile = !key.isEmpty() ? decryptJsonObject(encryptedFile, key) : QByteArray{};
         auto decryptedFileDoc = QJsonDocument::fromJson(decryptedFile);
         auto decryptedFileObj = decryptedFileDoc.object();
 
@@ -1503,6 +1514,11 @@ QByteArray FolderMetadata::decryptJsonObject(const QByteArray& encryptedMetadata
     return EncryptionHelper::decryptStringSymmetric(pass, encryptedMetadata);
 }
 
+bool FolderMetadata::isMetadataSetup() const
+{
+    return !_metadataKeys.isEmpty();
+}
+
 void FolderMetadata::setupEmptyMetadata() {
     qCDebug(lcCse) << "Settint up empty metadata";
     QByteArray newMetadataPass = EncryptionHelper::generateRandom(16);
@@ -1517,6 +1533,11 @@ void FolderMetadata::setupEmptyMetadata() {
 QByteArray FolderMetadata::encryptedMetadata() {
     qCDebug(lcCse) << "Generating metadata";
 
+    if (_metadataKeys.isEmpty()) {
+        qCDebug(lcCse) << "Metadata generation failed! Empty metadata key!";
+        return {};
+    }
+
     QJsonObject metadataKeys;
     for (auto it = _metadataKeys.constBegin(), end = _metadataKeys.constEnd(); it != end; it++) {
         /*
@@ -1527,16 +1548,6 @@ QByteArray FolderMetadata::encryptedMetadata() {
         metadataKeys.insert(QString::number(it.key()), QString(encryptedKey));
     }
 
-    /* NO SHARING IN V1
-    QJsonObject recepients;
-    for (auto it = _sharing.constBegin(), end = _sharing.constEnd(); it != end; it++) {
-        recepients.insert(it->first, it->second);
-    }
-    QJsonDocument recepientDoc;
-    recepientDoc.setObject(recepients);
-    QString sharingEncrypted = encryptJsonObject(recepientDoc.toJson(QJsonDocument::Compact), _metadataKeys.last());
-    */
-
     QJsonObject metadata = {
       {"metadataKeys", metadataKeys},
       // {"sharing", sharingEncrypted},

diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h
@@ -180,6 +180,7 @@ class OWNCLOUDSYNC_EXPORT FolderMetadata {
     void removeEncryptedFile(const EncryptedFile& f);
     void removeAllEncryptedFiles();
     QVector<EncryptedFile> files() const;
+    bool isMetadataSetup() const;
 
 
 private:

diff --git a/src/libsync/propagatedownloadencrypted.cpp b/src/libsync/propagatedownloadencrypted.cpp
@@ -73,18 +73,20 @@ void PropagateDownloadEncrypted::checkFolderEncryptedMetadata(const QJsonDocumen
   qCDebug(lcPropagateDownloadEncrypted) << "Metadata Received reading"
                                         << _item->_instruction << _item->_file << _item->_encryptedFileName;
   const QString filename = _info.fileName();
-  auto meta = new FolderMetadata(_propagator->account(), json.toJson(QJsonDocument::Compact));
-  const QVector<EncryptedFile> files = meta->files();
-
-  const QString encryptedFilename = _item->_encryptedFileName.section(QLatin1Char('/'), -1);
-  for (const EncryptedFile &file : files) {
-    if (encryptedFilename == file.encryptedFilename) {
-      _encryptedInfo = file;
-
-      qCDebug(lcPropagateDownloadEncrypted) << "Found matching encrypted metadata for file, starting download";
-      emit fileMetadataFound();
-      return;
-    }
+  const FolderMetadata metadata(_propagator->account(), json.toJson(QJsonDocument::Compact));
+  if (metadata.isMetadataSetup()) {
+      const QVector<EncryptedFile> files = metadata.files();
+
+      const QString encryptedFilename = _item->_encryptedFileName.section(QLatin1Char('/'), -1);
+      for (const EncryptedFile &file : files) {
+          if (encryptedFilename == file.encryptedFilename) {
+              _encryptedInfo = file;
+
+              qCDebug(lcPropagateDownloadEncrypted) << "Found matching encrypted metadata for file, starting download";
+              emit fileMetadataFound();
+              return;
+          }
+      }
   }
 
   emit failed();

diff --git a/src/libsync/propagateremotedeleteencrypted.cpp b/src/libsync/propagateremotedeleteencrypted.cpp
@@ -53,6 +53,11 @@ void PropagateRemoteDeleteEncrypted::slotFolderEncryptedMetadataReceived(const Q
 
     FolderMetadata metadata(_propagator->account(), json.toJson(QJsonDocument::Compact), statusCode);
 
+    if (!metadata.isMetadataSetup()) {
+        taskFailed();
+        return;
+    }
+
     qCDebug(PROPAGATE_REMOVE_ENCRYPTED) << "Metadata Received, preparing it for removal of the file";
 
     const QFileInfo info(_propagator->fullLocalPath(_item->_file));

diff --git a/src/libsync/propagateremotedeleteencryptedrootfolder.cpp b/src/libsync/propagateremotedeleteencryptedrootfolder.cpp
@@ -83,6 +83,11 @@ void PropagateRemoteDeleteEncryptedRootFolder::slotFolderEncryptedMetadataReceiv
 
     FolderMetadata metadata(_propagator->account(), json.toJson(QJsonDocument::Compact), statusCode);
 
+    if (!metadata.isMetadataSetup()) {
+        taskFailed();
+        return;
+    }
+
     qCDebug(PROPAGATE_REMOVE_ENCRYPTED_ROOTFOLDER) << "It's a root encrypted folder. Let's remove nested items first.";
 
     metadata.removeAllEncryptedFiles();

diff --git a/src/libsync/propagateuploadencrypted.cpp b/src/libsync/propagateuploadencrypted.cpp
@@ -122,7 +122,17 @@ void PropagateUploadEncrypted::slotFolderEncryptedMetadataReceived(const QJsonDo
   qCDebug(lcPropagateUploadEncrypted) << "Metadata Received, Preparing it for the new file." << json.toVariant();
 
   // Encrypt File!
-  _metadata = new FolderMetadata(_propagator->account(), json.toJson(QJsonDocument::Compact), statusCode);
+  _metadata.reset(new FolderMetadata(_propagator->account(), json.toJson(QJsonDocument::Compact), statusCode));
+
+  if (!_metadata->isMetadataSetup()) {
+      if (_isFolderLocked) {
+          connect(this, &PropagateUploadEncrypted::folderUnlocked, this, &PropagateUploadEncrypted::error);
+          unlockFolder();
+      } else {
+          emit error();
+      }
+      return;
+  }
 
   QFileInfo info(_propagator->fullLocalPath(_item->_file));
   const QString fileName = info.fileName();

diff --git a/src/libsync/propagateuploadencrypted.h b/src/libsync/propagateuploadencrypted.h
@@ -7,6 +7,7 @@
 #include <QByteArray>
 #include <QJsonDocument>
 #include <QNetworkReply>
+#include <QScopedPointer>
 #include <QFile>
 #include <QTemporaryFile>
 
@@ -76,7 +77,7 @@ private slots:
 
   QByteArray _generatedKey;
   QByteArray _generatedIv;
-  FolderMetadata *_metadata;
+  QScopedPointer<FolderMetadata> _metadata;
   EncryptedFile _encryptedFile;
   QString _completeFileName;
 };

diff --git a/src/libsync/vfs/cfapi/hydrationjob.cpp b/src/libsync/vfs/cfapi/hydrationjob.cpp
@@ -199,24 +199,27 @@ void OCC::HydrationJob::slotCheckFolderEncryptedMetadata(const QJsonDocument &js
     // TODO: the following code is borrowed from PropagateDownloadEncrypted (see HydrationJob::onNewConnection() for explanation of next steps)
     qCDebug(lcHydration) << "Metadata Received reading" << e2eMangledName();
     const QString filename = e2eMangledName();
-    auto meta = new FolderMetadata(_account, json.toJson(QJsonDocument::Compact));
-    const QVector<EncryptedFile> files = meta->files();
+    const FolderMetadata metadata(_account, json.toJson(QJsonDocument::Compact));
 
-    EncryptedFile encryptedInfo = {};
+    if (metadata.isMetadataSetup()) {
+        const QVector<EncryptedFile> files = metadata.files();
 
-    const QString encryptedFileExactName = e2eMangledName().section(QLatin1Char('/'), -1);
-    for (const EncryptedFile &file : files) {
-        if (encryptedFileExactName == file.encryptedFilename) {
-            EncryptedFile encryptedInfo = file;
-            encryptedInfo = file;
+        EncryptedFile encryptedInfo = {};
 
-            qCDebug(lcHydration) << "Found matching encrypted metadata for file, starting download" << _requestId << _folderPath;
-            _transferDataSocket = _transferDataServer->nextPendingConnection();
-            _job = new GETEncryptedFileJob(_account, _remotePath + e2eMangledName(), _transferDataSocket, {}, {}, 0, encryptedInfo, this);
+        const QString encryptedFileExactName = e2eMangledName().section(QLatin1Char('/'), -1);
+        for (const EncryptedFile &file : files) {
+            if (encryptedFileExactName == file.encryptedFilename) {
+                EncryptedFile encryptedInfo = file;
+                encryptedInfo = file;
 
-            connect(qobject_cast<GETEncryptedFileJob *>(_job), &GETEncryptedFileJob::finishedSignal, this, &HydrationJob::onGetFinished);
-            _job->start();
-            return;
+                qCDebug(lcHydration) << "Found matching encrypted metadata for file, starting download" << _requestId << _folderPath;
+                _transferDataSocket = _transferDataServer->nextPendingConnection();
+                _job = new GETEncryptedFileJob(_account, _remotePath + e2eMangledName(), _transferDataSocket, {}, {}, 0, encryptedInfo, this);
+
+                connect(qobject_cast<GETEncryptedFileJob *>(_job), &GETEncryptedFileJob::finishedSignal, this, &HydrationJob::onGetFinished);
+                _job->start();
+                return;
+            }
         }
     }