Improve resilience against memory attacks

To reduce residual fragments of secret data in memory after
deallocation, this patch replaces the global delete operator with a
version that zeros out previously allocated memory. It makes use of
the new C++14 sized deallocation, but provides an unsized fallback
with platform-specific size deductions.

This change is only a minor mitigation and cannot protect against
buffer reallocations by the operating system or non-C++ libraries.
Thus, we still cannot guarantee all memory to be wiped after free.

As a further improvement, this patch uses libgcrypt and libsodium
to write long-lived master key component hashes into a secure
memory area and wipe it afterwards.

The patch also fixes compiler flags not being set properly on macOS.

6 files changed, 93 insertions, 20 deletions

diff --git a/src/keys/FileKey.cpp b/src/keys/FileKey.cpp
index 9d1e8f50f..da25ef4ae 100644
--- a/src/keys/FileKey.cpp
+++ b/src/keys/FileKey.cpp
@@ -18,19 +18,35 @@
 
 #include "FileKey.h"
 
-#include <QFile>
-
 #include "core/Tools.h"
 #include "crypto/CryptoHash.h"
 #include "crypto/Random.h"
 
+#include <QFile>
+
+#include <sodium.h>
+#include <gcrypt.h>
+#include <algorithm>
+#include <cstring>
+
 QUuid FileKey::UUID("a584cbc4-c9b4-437e-81bb-362ca9709273");
 
+constexpr int FileKey::SHA256_SIZE;
+
 FileKey::FileKey()
     : Key(UUID)
+    , m_key(static_cast<char*>(gcry_malloc_secure(SHA256_SIZE)))
 {
 }
 
+FileKey::~FileKey()
+{
+    if (m_key) {
+        gcry_free(m_key);
+        m_key = nullptr;
+    }
+}
+
 /**
  * Read key file from device while trying to detect its file format.
  *
@@ -148,7 +164,10 @@ bool FileKey::load(const QString& fileName, QString* errorMsg)
  */
 QByteArray FileKey::rawKey() const
 {
-    return m_key;
+    if (!m_key) {
+        return {};
+    }
+    return QByteArray::fromRawData(m_key, SHA256_SIZE);
 }
 
 /**
@@ -223,12 +242,15 @@ bool FileKey::loadXml(QIODevice* device)
         }
     }
 
+    bool ok = false;
     if (!xmlReader.error() && correctMeta && !data.isEmpty()) {
-        m_key = data;
-        return true;
+        std::memcpy(m_key, data.data(), std::min(SHA256_SIZE, data.size()));
+        ok = true;
     }
 
-    return false;
+    sodium_memzero(data.data(), static_cast<std::size_t>(data.capacity()));
+
+    return ok;
 }
 
 /**
@@ -293,7 +315,8 @@ bool FileKey::loadBinary(QIODevice* device)
     if (!Tools::readAllFromDevice(device, data) || data.size() != 32) {
         return false;
     } else {
-        m_key = data;
+        std::memcpy(m_key, data.data(), std::min(SHA256_SIZE, data.size()));
+        sodium_memzero(data.data(), static_cast<std::size_t>(data.capacity()));
         return true;
     }
 }
@@ -321,12 +344,15 @@ bool FileKey::loadHex(QIODevice* device)
     }
 
     QByteArray key = QByteArray::fromHex(data);
+    sodium_memzero(data.data(), static_cast<std::size_t>(data.capacity()));
 
     if (key.size() != 32) {
         return false;
     }
 
-    m_key = key;
+    std::memcpy(m_key, key.data(), std::min(SHA256_SIZE, key.size()));
+    sodium_memzero(key.data(), static_cast<std::size_t>(key.capacity()));
+
     return true;
 }
 
@@ -348,7 +374,9 @@ bool FileKey::loadHashed(QIODevice* device)
         cryptoHash.addData(buffer);
     } while (!buffer.isEmpty());
 
-    m_key = cryptoHash.result();
+    auto result = cryptoHash.result();
+    std::memcpy(m_key, result.data(), std::min(SHA256_SIZE, result.size()));
+    sodium_memzero(result.data(), static_cast<std::size_t>(result.capacity()));
 
     return true;
 }
diff --git a/src/keys/FileKey.h b/src/keys/FileKey.h
index d7486467b..290a04af0 100644
--- a/src/keys/FileKey.h
+++ b/src/keys/FileKey.h
@@ -40,6 +40,7 @@ public:
     };
 
     FileKey();
+    ~FileKey() override;
     bool load(QIODevice* device);
     bool load(const QString& fileName, QString* errorMsg = nullptr);
     QByteArray rawKey() const override;
@@ -48,6 +49,8 @@ public:
     static bool create(const QString& fileName, QString* errorMsg = nullptr, int size = 128);
 
 private:
+    static constexpr int SHA256_SIZE = 32;
+
     bool loadXml(QIODevice* device);
     bool loadXmlMeta(QXmlStreamReader& xmlReader);
     QByteArray loadXmlKey(QXmlStreamReader& xmlReader);
@@ -55,7 +58,7 @@ private:
     bool loadHex(QIODevice* device);
     bool loadHashed(QIODevice* device);
 
-    QByteArray m_key;
+    char* m_key = nullptr;
     Type m_type = None;
 };
 
diff --git a/src/keys/PasswordKey.cpp b/src/keys/PasswordKey.cpp
index 35ecb9989..2d0416af8 100644
--- a/src/keys/PasswordKey.cpp
+++ b/src/keys/PasswordKey.cpp
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2010 Felix Geyer <debfx@fobos.de>
+ *  Copyright (C) 2019 KeePassXC Team <team@keepassxc.org>
  *
  *  This program is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -16,35 +16,51 @@
  */
 
 #include "PasswordKey.h"
+#include "core/Tools.h"
 
 #include "crypto/CryptoHash.h"
+#include <gcrypt.h>
+#include <algorithm>
+#include <cstring>
 
 QUuid PasswordKey::UUID("77e90411-303a-43f2-b773-853b05635ead");
 
+constexpr int PasswordKey::SHA256_SIZE;
+
 PasswordKey::PasswordKey()
     : Key(UUID)
+    , m_key(static_cast<char*>(gcry_malloc_secure(SHA256_SIZE)))
 {
 }
 
 PasswordKey::PasswordKey(const QString& password)
     : Key(UUID)
+    , m_key(static_cast<char*>(gcry_malloc_secure(SHA256_SIZE)))
 {
     setPassword(password);
 }
 
+PasswordKey::~PasswordKey()
+{
+    if (m_key) {
+        gcry_free(m_key);
+        m_key = nullptr;
+    }
+}
+
 QSharedPointer<PasswordKey> PasswordKey::fromRawKey(const QByteArray& rawKey)
 {
     auto result = QSharedPointer<PasswordKey>::create();
-    result->m_key = rawKey;
+    std::memcpy(result->m_key, rawKey.data(), std::min(SHA256_SIZE, rawKey.size()));
     return result;
 }
 
 QByteArray PasswordKey::rawKey() const
 {
-    return m_key;
+    return QByteArray::fromRawData(m_key, SHA256_SIZE);
 }
 
 void PasswordKey::setPassword(const QString& password)
 {
-    m_key = CryptoHash::hash(password.toUtf8(), CryptoHash::Sha256);
+    std::memcpy(m_key, CryptoHash::hash(password.toUtf8(), CryptoHash::Sha256).data(), SHA256_SIZE);
 }
diff --git a/src/keys/PasswordKey.h b/src/keys/PasswordKey.h
index 68ab79895..4408cabcf 100644
--- a/src/keys/PasswordKey.h
+++ b/src/keys/PasswordKey.h
@@ -30,13 +30,16 @@ public:
 
     PasswordKey();
     explicit PasswordKey(const QString& password);
+    ~PasswordKey() override;
     QByteArray rawKey() const override;
     void setPassword(const QString& password);
 
     static QSharedPointer<PasswordKey> fromRawKey(const QByteArray& rawKey);
 
 private:
-    QByteArray m_key;
+    static constexpr int SHA256_SIZE = 32;
+
+    char* m_key = nullptr;
 };
 
 #endif // KEEPASSX_PASSWORDKEY_H
diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp
index f9cbe3174..759d8d1bc 100644
--- a/src/keys/YkChallengeResponseKey.cpp
+++ b/src/keys/YkChallengeResponseKey.cpp
@@ -1,6 +1,6 @@
 /*
+ *  Copyright (C) 2019 KeePassXC Team <team@keepassxc.org>
  *  Copyright (C) 2014 Kyle Manna <kyle@kylemanna.com>
- *  Copyright (C) 2017 KeePassXC Team <team@keepassxc.org>
  *
  *  This program is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -32,6 +32,10 @@
 #include <QXmlStreamReader>
 #include <QtConcurrent>
 
+#include <gcrypt.h>
+#include <sodium.h>
+#include <cstring>
+
 QUuid YkChallengeResponseKey::UUID("e092495c-e77d-498b-84a1-05ae0d955508");
 
 YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking)
@@ -45,9 +49,18 @@ YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking)
     }
 }
 
+YkChallengeResponseKey::~YkChallengeResponseKey()
+{
+    if (m_key) {
+        gcry_free(m_key);
+        m_keySize = 0;
+        m_key = nullptr;
+    }
+}
+
 QByteArray YkChallengeResponseKey::rawKey() const
 {
-    return m_key;
+    return QByteArray::fromRawData(m_key, static_cast<int>(m_keySize));
 }
 
 /**
@@ -67,14 +80,22 @@ bool YkChallengeResponseKey::challenge(const QByteArray& challenge, unsigned int
             emit userInteractionRequired();
         }
 
+        QByteArray key;
         auto result = AsyncTask::runAndWaitForFuture(
-            [this, challenge]() { return YubiKey::instance()->challenge(m_slot, true, challenge, m_key); });
+            [this, challenge, &key]() { return YubiKey::instance()->challenge(m_slot, true, challenge, key); });
 
         if (m_blocking) {
             emit userConfirmed();
         }
 
         if (result == YubiKey::SUCCESS) {
+            if (m_key) {
+                gcry_free(m_key);
+            }
+            m_keySize = static_cast<std::size_t>(key.size());
+            m_key = static_cast<char*>(gcry_malloc_secure(m_keySize));
+            std::memcpy(m_key, key.data(), m_keySize);
+            sodium_memzero(key.data(), static_cast<std::size_t>(key.capacity()));
             return true;
         }
     } while (retries > 0);
diff --git a/src/keys/YkChallengeResponseKey.h b/src/keys/YkChallengeResponseKey.h
index b8467e7a6..5f7c40e72 100644
--- a/src/keys/YkChallengeResponseKey.h
+++ b/src/keys/YkChallengeResponseKey.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2017 KeePassXC Team <team@keepassxc.org>
+ *  Copyright (C) 2019 KeePassXC Team <team@keepassxc.org>
  *
  *  This program is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -32,6 +32,7 @@ public:
     static QUuid UUID;
 
     explicit YkChallengeResponseKey(int slot = -1, bool blocking = false);
+    ~YkChallengeResponseKey() override;
 
     QByteArray rawKey() const override;
     bool challenge(const QByteArray& challenge) override;
@@ -52,7 +53,8 @@ signals:
     void userConfirmed();
 
 private:
-    QByteArray m_key;
+    char* m_key = nullptr;
+    std::size_t m_keySize = 0;
     int m_slot;
     bool m_blocking;
 };