Fix crash on screen lock or computer sleep

* Fixes #10455
* Fixes #10432
* Fixes #10415

Prevent setting critical key components to nullptr when database data is cleared. This can result in a crash due to race condition between threads.

Added a bunch of asserts to detect this problem and if guards to prevent actual crashes.
This commit is contained in:
Jonathan White 2024-03-17 10:15:15 -04:00
parent f60601fa67
commit 6481ecccd7
5 changed files with 44 additions and 25 deletions

View File

@ -412,6 +412,9 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt
bool Database::writeDatabase(QIODevice* device, QString* error)
{
Q_ASSERT(m_data.key);
Q_ASSERT(m_data.transformedDatabaseKey);
PasswordKey oldTransformedKey;
if (m_data.key->isEmpty()) {
oldTransformedKey.setRawKey(m_data.transformedDatabaseKey->rawKey());
@ -767,18 +770,29 @@ Database::CompressionAlgorithm Database::compressionAlgorithm() const
QByteArray Database::transformedDatabaseKey() const
{
Q_ASSERT(m_data.transformedDatabaseKey);
if (!m_data.transformedDatabaseKey) {
return {};
}
return m_data.transformedDatabaseKey->rawKey();
}
QByteArray Database::challengeResponseKey() const
{
Q_ASSERT(m_data.challengeResponseKey);
if (!m_data.challengeResponseKey) {
return {};
}
return m_data.challengeResponseKey->rawKey();
}
bool Database::challengeMasterSeed(const QByteArray& masterSeed)
{
Q_ASSERT(m_data.key);
Q_ASSERT(m_data.masterSeed);
m_keyError.clear();
if (m_data.key) {
if (m_data.key && m_data.masterSeed) {
m_data.masterSeed->setRawKey(masterSeed);
QByteArray response;
bool ok = m_data.key->challenge(masterSeed, response, &m_keyError);
@ -824,8 +838,7 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
m_keyError.clear();
if (!key) {
m_data.key.reset();
m_data.transformedDatabaseKey.reset(new PasswordKey());
m_data.resetKeys();
return true;
}

View File

@ -188,30 +188,33 @@ private:
QScopedPointer<PasswordKey> challengeResponseKey;
QSharedPointer<const CompositeKey> key;
QSharedPointer<Kdf> kdf = QSharedPointer<AesKdf>::create(true);
QSharedPointer<Kdf> kdf;
QVariantMap publicCustomData;
DatabaseData()
: masterSeed(new PasswordKey())
, transformedDatabaseKey(new PasswordKey())
, challengeResponseKey(new PasswordKey())
{
kdf->randomizeSeed();
clear();
}
void clear()
{
resetKeys();
filePath.clear();
publicCustomData.clear();
}
masterSeed.reset();
transformedDatabaseKey.reset();
challengeResponseKey.reset();
void resetKeys()
{
masterSeed.reset(new PasswordKey());
transformedDatabaseKey.reset(new PasswordKey());
challengeResponseKey.reset(new PasswordKey());
key.reset();
kdf.reset();
publicCustomData.clear();
// Default to AES KDF, KDBX4 databases overwrite this
kdf.reset(new AesKdf(true));
kdf->randomizeSeed();
}
};

View File

@ -264,9 +264,7 @@ void DatabaseOpenWidget::clearForms()
m_ui->hardwareKeyCombo->clear();
toggleQuickUnlockScreen();
QString error;
m_db.reset(new Database());
m_db->open(m_filename, nullptr, &error);
m_db.reset(new Database(m_filename));
}
QSharedPointer<Database> DatabaseOpenWidget::database()

View File

@ -101,22 +101,23 @@ void DatabaseSettingsWidgetEncryption::initialize()
}
auto version = KDBX4;
if (m_db->kdf()) {
if (m_db->key() && m_db->kdf()) {
version = (m_db->kdf()->uuid() == KeePass2::KDF_AES_KDBX3) ? KDBX3 : KDBX4;
}
m_ui->compatibilitySelection->setCurrentIndex(version);
bool isNewDatabase = false;
if (!m_db->kdf()) {
if (!m_db->key()) {
m_db->setKey(QSharedPointer<CompositeKey>::create(), true, false, false);
m_db->setKdf(KeePass2::uuidToKdf(KeePass2::KDF_ARGON2D));
m_db->setCipher(KeePass2::CIPHER_AES256);
isNewDatabase = true;
} else if (!m_db->kdf()) {
m_db->setKdf(KeePass2::uuidToKdf(KeePass2::KDF_ARGON2D));
isNewDatabase = true;
}
if (!m_db->key()) {
m_db->setKey(QSharedPointer<CompositeKey>::create(), true, false, false);
m_db->setCipher(KeePass2::CIPHER_AES256);
isNewDatabase = true;
}
bool kdbx3Enabled = KeePass2Writer::kdbxVersionRequired(m_db.data(), true, true) <= KeePass2::FILE_VERSION_3_1;
// check if the DB's custom data has a decryption time setting stored

View File

@ -582,7 +582,9 @@ void TestKeePass2Format::testKdbxKeyChange()
db->setKey(key1);
writeKdbx(&buffer, db.data(), hasError, errorString);
QVERIFY(!hasError);
if (hasError) {
QFAIL(qPrintable(QStringLiteral("Error while reading database: ").append(errorString)));
}
// read database
db = QSharedPointer<Database>::create();
@ -599,7 +601,9 @@ void TestKeePass2Format::testKdbxKeyChange()
// write database
buffer.seek(0);
writeKdbx(&buffer, db.data(), hasError, errorString);
QVERIFY(!hasError);
if (hasError) {
QFAIL(qPrintable(QStringLiteral("Error while reading database: ").append(errorString)));
}
// read database
db = QSharedPointer<Database>::create();