Fix security vulnerability when receiving empty metadataKeys from the server.
authoralex-z <blackslayer4@gmail.com>
Wed, 11 Jan 2023 17:25:01 +0000 (18:25 +0100)
committerallexzander <allexzander@users.noreply.github.com>
Wed, 18 Jan 2023 10:41:55 +0000 (11:41 +0100)
Signed-off-by: alex-z <blackslayer4@gmail.com>
src/libsync/clientsideencryption.cpp
src/libsync/clientsideencryption.h
src/libsync/propagatedownloadencrypted.cpp
src/libsync/propagateremotedeleteencrypted.cpp
src/libsync/propagateremotedeleteencryptedrootfolder.cpp
src/libsync/propagateuploadencrypted.cpp
src/libsync/vfs/cfapi/hydrationjob.cpp

index f3b0af87b9c2dada9e13e2fb6a6d28a0aada30ef..762dd5b2a79e994e909d9d334dc3a0efbf4fdb2f 100644 (file)
@@ -1419,6 +1419,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();
 
@@ -1447,14 +1453,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";
@@ -1470,9 +1481,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();
 
@@ -1532,6 +1543,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);
@@ -1546,6 +1562,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++) {
         /*
@@ -1556,16 +1577,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},
index 552282e3864c9bd6d8c1cf718bb4f20829cd7659..664e92ad30b9443d66a42c9b2e34235c09a673fa 100644 (file)
@@ -183,6 +183,7 @@ public:
     void removeEncryptedFile(const EncryptedFile& f);
     void removeAllEncryptedFiles();
     [[nodiscard]] QVector<EncryptedFile> files() const;
+    [[nodiscard]] bool isMetadataSetup() const;
 
 
 private:
index 74f671cd78e26f37c57dd1c48e1ddd9256e72afa..6543c58adf9565ae86d1fc3a34f2c7e06043c3f6 100644 (file)
@@ -74,17 +74,19 @@ void PropagateDownloadEncrypted::checkFolderEncryptedMetadata(const QJsonDocumen
                                         << _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;
-    }
+  if (meta->isMetadataSetup()) {
+      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;
+          }
+      }
   }
 
   emit failed();
index c648d79e6bffabbf89e3210018eb6e6a384d8d37..1d7ae8aa6c47f1016496bb443d72757724a8f772 100644 (file)
@@ -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));
index 3985f530831e1eadb631033bb118550d09c03f9d..e8d86e30811287d5a61b9c1a208617fa75085eac 100644 (file)
@@ -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();
index b81f45b15ba61976821d124e82181e2183a6fd01..de61df307d96bd049b06c645d46fdd13d8e29e66 100644 (file)
@@ -120,10 +120,19 @@ void PropagateUploadEncrypted::slotFolderEncryptedMetadataError(const QByteArray
 void PropagateUploadEncrypted::slotFolderEncryptedMetadataReceived(const QJsonDocument &json, int statusCode)
 {
   qCDebug(lcPropagateUploadEncrypted) << "Metadata Received, Preparing it for the new file." << json.toVariant();
-
   // Encrypt File!
   _metadata = 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();
 
@@ -197,15 +206,13 @@ void PropagateUploadEncrypted::slotFolderEncryptedMetadataReceived(const QJsonDo
 
   if (statusCode == 404) {
     auto job = new StoreMetaDataApiJob(_propagator->account(),
-                                       _folderId,
-                                       _metadata->encryptedMetadata());
+                                       _folderId, _metadata->encryptedMetadata());
     connect(job, &StoreMetaDataApiJob::success, this, &PropagateUploadEncrypted::slotUpdateMetadataSuccess);
     connect(job, &StoreMetaDataApiJob::error, this, &PropagateUploadEncrypted::slotUpdateMetadataError);
     job->start();
   } else {
     auto job = new UpdateMetadataApiJob(_propagator->account(),
-                                      _folderId,
-                                      _metadata->encryptedMetadata(),
+                                      _folderId, _metadata->encryptedMetadata(),
                                       _folderToken);
 
     connect(job, &UpdateMetadataApiJob::success, this, &PropagateUploadEncrypted::slotUpdateMetadataSuccess);
index 26c8a19e74233e15d179c9979e2b61a058ec5cb3..754c2183bb6ba511e2bf9c670104a589393be898 100644 (file)
@@ -200,23 +200,26 @@ void OCC::HydrationJob::slotCheckFolderEncryptedMetadata(const QJsonDocument &js
     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();
 
-    EncryptedFile encryptedInfo = {};
+    if (meta->isMetadataSetup()) {
+        const QVector<EncryptedFile> files = meta->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;
+            }
         }
     }