From 28e6887aa4db16ba9cd4b875f8d547cec99fd539 Mon Sep 17 00:00:00 2001
From: Jonathan White <support@dmapps.us>
Date: Thu, 23 May 2024 07:02:55 -0400
Subject: [PATCH] Improve handling of encrypted Bitwarden json

* Fixes #10785
---
 share/translations/keepassxc_en.ts | 12 ++++++++++--
 src/format/BitwardenReader.cpp     | 18 +++++++++++++++---
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts
index 278db7e59..d988d6a55 100644
--- a/share/translations/keepassxc_en.ts
+++ b/share/translations/keepassxc_en.ts
@@ -8871,11 +8871,19 @@ This option is deprecated, use --set-key-file instead.</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <source>Unsupported KDF type, cannot decrypt json file</source>
+        <source>Unknown passkeys error</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <source>Unknown passkeys error</source>
+        <source>Invalid KDF iterations, cannot decrypt json file</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Unsupported format, ensure your Bitwarden export is password-protected</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Only PBKDF and Argon2 are supported, cannot decrypt json file</source>
         <translation type="unfinished"></translation>
     </message>
 </context>
diff --git a/src/format/BitwardenReader.cpp b/src/format/BitwardenReader.cpp
index 8f9d86b8f..861169b7b 100644
--- a/src/format/BitwardenReader.cpp
+++ b/src/format/BitwardenReader.cpp
@@ -252,14 +252,25 @@ QSharedPointer<Database> BitwardenReader::convert(const QString& path, const QSt
             return QObject::tr("Failed to decrypt json file: %1").arg(errorString);
         };
 
+        if (!json.contains("kdfType") || !json.contains("salt")) {
+            m_error = buildError(QObject::tr("Unsupported format, ensure your Bitwarden export is password-protected"));
+            return {};
+        }
+
         QByteArray key(32, '\0');
         auto salt = json.value("salt").toString().toUtf8();
         auto kdfType = json.value("kdfType").toInt();
 
         // Derive the Master Key
         if (kdfType == 0) {
+            // PBKDF2
+            auto iterations = json.value("kdfIterations").toInt();
+            if (iterations <= 0) {
+                m_error = buildError(QObject::tr("Invalid KDF iterations, cannot decrypt json file"));
+                return {};
+            }
             auto pwd_fam = Botan::PasswordHashFamily::create_or_throw("PBKDF2(SHA-256)");
-            auto pwd_hash = pwd_fam->from_params(json.value("kdfIterations").toInt());
+            auto pwd_hash = pwd_fam->from_params(iterations);
             pwd_hash->derive_key(reinterpret_cast<uint8_t*>(key.data()),
                                  key.size(),
                                  password.toUtf8().data(),
@@ -267,7 +278,8 @@ QSharedPointer<Database> BitwardenReader::convert(const QString& path, const QSt
                                  reinterpret_cast<uint8_t*>(salt.data()),
                                  salt.size());
         } else if (kdfType == 1) {
-            // Bitwarden hashes the salt for Argon2 for some reason
+            // Argon2
+            // Bitwarden hashes the salt prior to use
             CryptoHash saltHash(CryptoHash::Sha256);
             saltHash.addData(salt);
             salt = saltHash.result();
@@ -279,7 +291,7 @@ QSharedPointer<Database> BitwardenReader::convert(const QString& path, const QSt
             argon2.setParallelism(json.value("kdfParallelism").toInt());
             argon2.transform(password.toUtf8(), key);
         } else {
-            m_error = buildError(QObject::tr("Unsupported KDF type, cannot decrypt json file"));
+            m_error = buildError(QObject::tr("Only PBKDF and Argon2 are supported, cannot decrypt json file"));
             return {};
         }