From eb56bd8973859e49384ada8757639466f483c22c Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Thu, 28 Jan 2016 23:07:04 +0100 Subject: [PATCH] Add repair functionality to strip invalid XML chars. Refs #392 --- src/CMakeLists.txt | 3 + src/format/KeePass2Reader.cpp | 14 +++- src/format/KeePass2Reader.h | 3 +- src/format/KeePass2Repair.cpp | 107 +++++++++++++++++++++++++++++++ src/format/KeePass2Repair.h | 50 +++++++++++++++ src/gui/DatabaseRepairWidget.cpp | 103 +++++++++++++++++++++++++++++ src/gui/DatabaseRepairWidget.h | 41 ++++++++++++ src/gui/MainWindow.cpp | 37 +++++++++++ src/gui/MainWindow.h | 1 + src/gui/MainWindow.ui | 6 ++ tests/TestKeePass2Writer.cpp | 30 +++++++++ tests/TestKeePass2Writer.h | 1 + tests/data/bug392.kdbx | Bin 0 -> 1374 bytes 13 files changed, 393 insertions(+), 3 deletions(-) create mode 100644 src/format/KeePass2Repair.cpp create mode 100644 src/format/KeePass2Repair.h create mode 100644 src/gui/DatabaseRepairWidget.cpp create mode 100644 src/gui/DatabaseRepairWidget.h create mode 100644 tests/data/bug392.kdbx diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1270d346..da8b9ec1 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -78,6 +78,7 @@ set(keepassx_SOURCES format/KeePass2.h format/KeePass2RandomStream.cpp format/KeePass2Reader.cpp + format/KeePass2Repair.cpp format/KeePass2Writer.cpp format/KeePass2XmlReader.cpp format/KeePass2XmlWriter.cpp @@ -86,6 +87,7 @@ set(keepassx_SOURCES gui/ChangeMasterKeyWidget.cpp gui/Clipboard.cpp gui/DatabaseOpenWidget.cpp + gui/DatabaseRepairWidget.cpp gui/DatabaseSettingsWidget.cpp gui/DatabaseTabWidget.cpp gui/DatabaseWidget.cpp @@ -180,6 +182,7 @@ set(keepassx_MOC gui/ChangeMasterKeyWidget.h gui/Clipboard.h gui/DatabaseOpenWidget.h + gui/DatabaseRepairWidget.h gui/DatabaseSettingsWidget.h gui/DatabaseTabWidget.h gui/DatabaseWidget.h diff --git a/src/format/KeePass2Reader.cpp b/src/format/KeePass2Reader.cpp index adde8cdb..d1ca9ed9 100644 --- a/src/format/KeePass2Reader.cpp +++ b/src/format/KeePass2Reader.cpp @@ -43,7 +43,7 @@ KeePass2Reader::KeePass2Reader() { } -Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& key) +Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& key, bool keepDatabase) { QScopedPointer db(new Database()); m_db = db.data(); @@ -178,7 +178,12 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& ke if (xmlReader.hasError()) { raiseError(xmlReader.errorString()); - return Q_NULLPTR; + if (keepDatabase) { + return db.take(); + } + else { + return Q_NULLPTR; + } } Q_ASSERT(version < 0x00030001 || !xmlReader.headerHash().isEmpty()); @@ -232,6 +237,11 @@ QByteArray KeePass2Reader::xmlData() return m_xmlData; } +QByteArray KeePass2Reader::streamKey() +{ + return m_protectedStreamKey; +} + void KeePass2Reader::raiseError(const QString& errorMessage) { m_error = true; diff --git a/src/format/KeePass2Reader.h b/src/format/KeePass2Reader.h index 2c9daab2..827e671c 100644 --- a/src/format/KeePass2Reader.h +++ b/src/format/KeePass2Reader.h @@ -31,12 +31,13 @@ class KeePass2Reader public: KeePass2Reader(); - Database* readDatabase(QIODevice* device, const CompositeKey& key); + Database* readDatabase(QIODevice* device, const CompositeKey& key, bool keepDatabase = false); Database* readDatabase(const QString& filename, const CompositeKey& key); bool hasError(); QString errorString(); void setSaveXml(bool save); QByteArray xmlData(); + QByteArray streamKey(); private: void raiseError(const QString& errorMessage); diff --git a/src/format/KeePass2Repair.cpp b/src/format/KeePass2Repair.cpp new file mode 100644 index 00000000..4eff3d80 --- /dev/null +++ b/src/format/KeePass2Repair.cpp @@ -0,0 +1,107 @@ +/* + * Copyright (C) 2016 Felix Geyer + * + * 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 + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "KeePass2Repair.h" + +#include +#include + +#include "format/KeePass2RandomStream.h" +#include "format/KeePass2Reader.h" +#include "format/KeePass2XmlReader.h" + +KeePass2Repair::KeePass2Repair() + : m_db(Q_NULLPTR) +{ +} + +KeePass2Repair::RepairResult KeePass2Repair::repairDatabase(QIODevice* device, const CompositeKey& key) +{ + m_db = Q_NULLPTR; + m_errorStr.clear(); + + KeePass2Reader reader; + reader.setSaveXml(true); + + Database* db = reader.readDatabase(device, key, true); + if (!reader.hasError()) { + delete db; + return NothingTodo; + } + + QByteArray xmlData = reader.xmlData(); + if (!db || xmlData.isEmpty()) { + delete db; + m_errorStr = reader.errorString(); + return UnableToOpen; + } + + bool repairAction = false; + + QString xmlStart = QString::fromLatin1(xmlData.constData(), qMin(100, xmlData.size())); + QRegExp encodingRegExp("encoding=\"([^\"]+)\"", Qt::CaseInsensitive, QRegExp::RegExp2); + if (encodingRegExp.indexIn(xmlStart) != -1) { + if (encodingRegExp.cap(1).compare("utf-8", Qt::CaseInsensitive) != 0 + && encodingRegExp.cap(1).compare("utf8", Qt::CaseInsensitive) != 0) + { + // database is not utf-8 encoded, we don't support repairing that + delete db; + return RepairFailed; + } + } + + // try to fix broken databases because of bug #392 + for (int i = (xmlData.size() - 1); i >= 0; i--) { + char ch = xmlData.at(i); + if (ch < 0x20 && ch != 0x09 && ch != 0x0A && ch != 0x0D) { + xmlData.remove(i, 1); + repairAction = true; + } + } + + if (!repairAction) { + // we were unable to find the problem + delete db; + return RepairFailed; + } + + KeePass2RandomStream randomStream; + randomStream.init(reader.streamKey()); + KeePass2XmlReader xmlReader; + QBuffer buffer(&xmlData); + buffer.open(QIODevice::ReadOnly); + xmlReader.readDatabase(&buffer, db, &randomStream); + + if (xmlReader.hasError()) { + delete db; + return RepairFailed; + } + else { + m_db = db; + return RepairSuccess; + } +} + +Database* KeePass2Repair::database() const +{ + return m_db; +} + +QString KeePass2Repair::errorString() const +{ + return m_errorStr; +} diff --git a/src/format/KeePass2Repair.h b/src/format/KeePass2Repair.h new file mode 100644 index 00000000..fe2f9dbf --- /dev/null +++ b/src/format/KeePass2Repair.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2016 Felix Geyer + * + * 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 + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSX_KEEPASS2REPAIR_H +#define KEEPASSX_KEEPASS2REPAIR_H + +#include +#include + +#include "core/Database.h" +#include "keys/CompositeKey.h" + +class KeePass2Repair +{ + Q_DECLARE_TR_FUNCTIONS(KeePass2Repair) + +public: + enum RepairResult + { + NothingTodo, + UnableToOpen, + RepairSuccess, + RepairFailed + }; + + KeePass2Repair(); + RepairResult repairDatabase(QIODevice* device, const CompositeKey& key); + Database* database() const; + QString errorString() const; + +private: + Database* m_db; + QString m_errorStr; +}; + +#endif // KEEPASSX_KEEPASS2REPAIR_H diff --git a/src/gui/DatabaseRepairWidget.cpp b/src/gui/DatabaseRepairWidget.cpp new file mode 100644 index 00000000..b7eeac21 --- /dev/null +++ b/src/gui/DatabaseRepairWidget.cpp @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2016 Felix Geyer + * + * 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 + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "DatabaseRepairWidget.h" + +#include +#include + +#include "ui_DatabaseOpenWidget.h" +#include "core/Database.h" +#include "core/Metadata.h" +#include "format/KeePass2Repair.h" +#include "gui/MessageBox.h" +#include "keys/FileKey.h" +#include "keys/PasswordKey.h" + +DatabaseRepairWidget::DatabaseRepairWidget(QWidget* parent) + : DatabaseOpenWidget(parent) +{ + m_ui->labelHeadline->setText(tr("Repair database")); + + connect(this, SIGNAL(editFinished(bool)), this, SLOT(processEditFinished(bool))); +} + +void DatabaseRepairWidget::openDatabase() +{ + CompositeKey masterKey; + + if (m_ui->checkPassword->isChecked()) { + masterKey.addKey(PasswordKey(m_ui->editPassword->text())); + } + + if (m_ui->checkKeyFile->isChecked()) { + FileKey key; + QString keyFilename = m_ui->comboKeyFile->currentText(); + QString errorMsg; + if (!key.load(keyFilename, &errorMsg)) { + MessageBox::warning(this, tr("Error"), tr("Can't open key file").append(":\n").append(errorMsg)); + Q_EMIT editFinished(false); + } + masterKey.addKey(key); + } + + KeePass2Repair repair; + + QFile file(m_filename); + if (!file.open(QIODevice::ReadOnly)) { + // TODO: error message + Q_EMIT editFinished(false); + return; + } + if (m_db) { + delete m_db; + } + QApplication::setOverrideCursor(QCursor(Qt::WaitCursor)); + KeePass2Repair::RepairResult repairResult = repair.repairDatabase(&file, masterKey); + QApplication::restoreOverrideCursor(); + + switch (repairResult) { + case KeePass2Repair::NothingTodo: + MessageBox::information(this, tr("Error"), tr("Database opened fine. Nothing to do.")); + Q_EMIT editFinished(false); + return; + case KeePass2Repair::UnableToOpen: + MessageBox::warning(this, tr("Error"), tr("Unable to open the database.").append("\n") + .append(repair.errorString())); + Q_EMIT editFinished(false); + return; + case KeePass2Repair::RepairSuccess: + m_db = repair.database(); + MessageBox::warning(this, tr("Success"), tr("The database has been successfully repaired\nYou can now save it.")); + Q_EMIT editFinished(true); + return; + case KeePass2Repair::RepairFailed: + MessageBox::warning(this, tr("Error"), tr("Unable to repair the database.")); + Q_EMIT editFinished(false); + return; + } +} + +void DatabaseRepairWidget::processEditFinished(bool result) +{ + if (result) { + Q_EMIT success(); + } + else { + Q_EMIT error(); + } +} diff --git a/src/gui/DatabaseRepairWidget.h b/src/gui/DatabaseRepairWidget.h new file mode 100644 index 00000000..56c5c960 --- /dev/null +++ b/src/gui/DatabaseRepairWidget.h @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2016 Felix Geyer + * + * 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 + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSX_DATABASEREPAIRWIDGET_H +#define KEEPASSX_DATABASEREPAIRWIDGET_H + +#include "gui/DatabaseOpenWidget.h" + +class DatabaseRepairWidget : public DatabaseOpenWidget +{ + Q_OBJECT + +public: + explicit DatabaseRepairWidget(QWidget* parent = Q_NULLPTR); + +Q_SIGNALS: + void success(); + void error(); + +protected: + void openDatabase() Q_DECL_OVERRIDE; + +private Q_SLOTS: + void processEditFinished(bool result); +}; + +#endif // KEEPASSX_DATABASEREPAIRWIDGET_H diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index cf26f181..b6dc1df1 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -27,8 +27,12 @@ #include "core/FilePath.h" #include "core/InactivityTimer.h" #include "core/Metadata.h" +#include "format/KeePass2Writer.h" #include "gui/AboutDialog.h" #include "gui/DatabaseWidget.h" +#include "gui/DatabaseRepairWidget.h" +#include "gui/FileDialog.h" +#include "gui/MessageBox.h" const QString MainWindow::BaseWindowTitle = "KeePassX"; @@ -163,6 +167,8 @@ MainWindow::MainWindow() SLOT(changeDatabaseSettings())); connect(m_ui->actionImportKeePass1, SIGNAL(triggered()), m_ui->tabWidget, SLOT(importKeePass1Database())); + connect(m_ui->actionRepairDatabase, SIGNAL(triggered()), this, + SLOT(repairDatabase())); connect(m_ui->actionExportCsv, SIGNAL(triggered()), m_ui->tabWidget, SLOT(exportToCsv())); connect(m_ui->actionLockDatabases, SIGNAL(triggered()), m_ui->tabWidget, @@ -366,6 +372,7 @@ void MainWindow::setMenuActionState(DatabaseWidget::Mode mode) m_ui->actionDatabaseOpen->setEnabled(inDatabaseTabWidgetOrWelcomeWidget); m_ui->menuRecentDatabases->setEnabled(inDatabaseTabWidgetOrWelcomeWidget); m_ui->actionImportKeePass1->setEnabled(inDatabaseTabWidgetOrWelcomeWidget); + m_ui->actionRepairDatabase->setEnabled(inDatabaseTabWidgetOrWelcomeWidget); m_ui->actionLockDatabases->setEnabled(m_ui->tabWidget->hasLockableDatabases()); } @@ -598,6 +605,36 @@ void MainWindow::lockDatabasesAfterInactivity() m_ui->tabWidget->lockDatabases(); } +void MainWindow::repairDatabase() +{ + QString filter = QString("%1 (*.kdbx);;%2 (*)").arg(tr("KeePass 2 Database"), tr("All files")); + QString fileName = fileDialog()->getOpenFileName(this, tr("Open database"), QString(), + filter); + if (fileName.isEmpty()) { + return; + } + + QScopedPointer dialog(new QDialog(this)); + DatabaseRepairWidget* dbRepairWidget = new DatabaseRepairWidget(dialog.data()); + connect(dbRepairWidget, SIGNAL(success()), dialog.data(), SLOT(accept())); + connect(dbRepairWidget, SIGNAL(error()), dialog.data(), SLOT(reject())); + dbRepairWidget->load(fileName); + if (dialog->exec() == QDialog::Accepted && dbRepairWidget->database()) { + QString saveFileName = fileDialog()->getSaveFileName(this, tr("Save repaired database"), QString(), + tr("KeePass 2 Database").append(" (*.kdbx)"), + Q_NULLPTR, 0, "kdbx"); + + if (!saveFileName.isEmpty()) { + KeePass2Writer writer; + writer.writeDatabase(saveFileName, dbRepairWidget->database()); + if (writer.hasError()) { + MessageBox::critical(this, tr("Error"), tr("Writing the database failed.") + "\n\n" + + writer.errorString()); + } + } + } +} + bool MainWindow::isTrayIconEnabled() const { #ifdef Q_OS_MAC diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h index 695c20d4..5aa39ca0 100644 --- a/src/gui/MainWindow.h +++ b/src/gui/MainWindow.h @@ -66,6 +66,7 @@ private Q_SLOTS: void trayIconTriggered(QSystemTrayIcon::ActivationReason reason); void toggleWindow(); void lockDatabasesAfterInactivity(); + void repairDatabase(); private: static void setShortcut(QAction* action, QKeySequence::StandardKey standard, int fallback = 0); diff --git a/src/gui/MainWindow.ui b/src/gui/MainWindow.ui index 92c80634..66c040e9 100644 --- a/src/gui/MainWindow.ui +++ b/src/gui/MainWindow.ui @@ -121,6 +121,7 @@ + @@ -422,6 +423,11 @@ Export to CSV file + + + Repair database + + diff --git a/tests/TestKeePass2Writer.cpp b/tests/TestKeePass2Writer.cpp index fd478a87..638d4002 100644 --- a/tests/TestKeePass2Writer.cpp +++ b/tests/TestKeePass2Writer.cpp @@ -20,6 +20,7 @@ #include #include +#include "config-keepassx-tests.h" #include "tests.h" #include "FailDevice.h" #include "core/Database.h" @@ -27,6 +28,7 @@ #include "core/Metadata.h" #include "crypto/Crypto.h" #include "format/KeePass2Reader.h" +#include "format/KeePass2Repair.h" #include "format/KeePass2Writer.h" #include "format/KeePass2XmlWriter.h" #include "keys/PasswordKey.h" @@ -127,6 +129,34 @@ void TestKeePass2Writer::testDeviceFailure() delete db; } +void TestKeePass2Writer::testRepair() +{ + QString brokenDbFilename = QString(KEEPASSX_TEST_DATA_DIR).append("/bug392.kdbx"); + // master password = test + // entry username: testuser\x10 + // entry password: testpw + CompositeKey key; + key.addKey(PasswordKey("test")); + + // test that we can't open the broken database + KeePass2Reader reader; + Database* dbBroken = reader.readDatabase(brokenDbFilename, key); + QVERIFY(!dbBroken); + QVERIFY(reader.hasError()); + + // test if we can repair the database + KeePass2Repair repair; + QFile file(brokenDbFilename); + file.open(QIODevice::ReadOnly); + QCOMPARE(repair.repairDatabase(&file, key), KeePass2Repair::RepairSuccess); + Database* dbRepaired = repair.database(); + QVERIFY(dbRepaired); + + QCOMPARE(dbRepaired->rootGroup()->entries().size(), 1); + QCOMPARE(dbRepaired->rootGroup()->entries().at(0)->username(), QString("testuser")); + QCOMPARE(dbRepaired->rootGroup()->entries().at(0)->password(), QString("testpw")); +} + void TestKeePass2Writer::cleanupTestCase() { delete m_dbOrg; diff --git a/tests/TestKeePass2Writer.h b/tests/TestKeePass2Writer.h index eb3f1095..82288382 100644 --- a/tests/TestKeePass2Writer.h +++ b/tests/TestKeePass2Writer.h @@ -33,6 +33,7 @@ private Q_SLOTS: void testAttachments(); void testNonAsciiPasswords(); void testDeviceFailure(); + void testRepair(); void cleanupTestCase(); private: diff --git a/tests/data/bug392.kdbx b/tests/data/bug392.kdbx new file mode 100644 index 0000000000000000000000000000000000000000..c649f8dc240e704f65b36e7127144e75cc77a10b GIT binary patch literal 1374 zcmV-k1)=%_*`k_f`%AR}00RI55CAd3^5(yBLr}h01tDtuTK@wC0096100bZaI3;bW zT9+*$Y75=Zd7VSD_ah^)Vpzc&(j&9umm)}J1t0)xcKq_FJwh6x_fz4qKgcSEwh~vH z7IRQ(1ofU88op}=2mqjl0RR91000LN03jz_4Q$!y$eU#Y`b0KZlL#OHOiVvnkbA#z zo>(B(oZK=k#JtoLN0^~q!p4`U&0|O`2_OKNoGKRWn`B)~xf(CJ1$Lz<2enOBTiLH@ z0=&>y9AHEW1ONg60000401XNa3T97g9;>B_Oi(w&&5GR6puQpbNv;8hugv#m30jt> z>zUM*f_}_WNR63iSBq_tqE%!z0#Uh8RROD$67psz+lk@PM72ihoUf5toVhN{5~p>5 zHCy;wS5c4dY7?2uBOxIk6j#oC(;U24nhub31qGg3EjC0g5FIVJksDRg=c~Yxt-EJp{ z2<`sx!*Pj6p5&Rg%-y{3*A3-A1`qp&7TbN8Rs3ws`aJ}XWyZT26A~&;r?a-VarJ`g zVm1CFbaa6!Wa)Z*6p@0z-}ee~^Zwc4`Kfz;Nw^$^fIoC1G;nk$!N6g#4>VlA#BpqP zts0w6r_q2)7GQRq01OmSSkK4Z3EK3ZzJxLg0FtB1JYkooiPqG7v&%gvfHwtx{s}$0 z)qqTyL<|yFH`j8A=j0n>tl|4H{rHkR6if2Q7?HiIb%sm2V-AEUB$kAG?TPJ@EUGV% zwVXaH63YaRo}RCZypj{sVKfwELRb~jGgOzZ9MIJZI)GefqZVyjbPfft;!&*U>T&6Z z-mY;JV%X@7-SY0#e;{)rVlm}nbQ8CelTxUbH5b+H1d%O#+%fZ)WK7NY3#O~GvbxE% zj%;{PL_~S)7gqDgu6_b{irgxb$xB;w+In!N#;Ne?RIS6}QKYT=j0p2YQU6M(x?Dr1q%-%CV;#luwzN_iacabOx2 zsaPv$I#@{~m>73KygCZ%Zrqj@k(Q#M_jh)MSx%I9KcY`#{|9(OdhDY()Gu*Y3fHV3 zx@Wygw>?@SYGr|$G37jzH0@SjqdkU&PZz*jcOb1|Q~kq>2BrJa^ezKfmI!_XTc-ii z1*O`s7T&|1j|LlovS)Z2rX0^B~x?96)Z9u5(OOWms0 zJ?A9dQmNU22HR5!-VRF(_axnbMN^Em_U!7)jAH=E^bO})Fw|?F3+D8)VI-i8Xc;b3 zy;2V@P94j;^kTc(>Jo6C0gaTdTzPwocOuNYunl!ZM>cix=IhS$9I0@i(S2;Fblo@+ z%F>O@-a(r3`nKD_5VylPJGho{qq}$-1s*n%;?zj1UWr94q#X{FU7Y654m2Rc3>CRaOxdMHVZ!+4VL9ZdcaXBC zst7ix6g$19VR*DrSz?+_NMb4nNsx;44sv5sdrnAo;NYY%fNG7y%M*=-DIUdIv=H|K gQ2u&UW1v_&1L9Z3*^bcTR7=uutm$R3ME0rOx$&-!tpET3 literal 0 HcmV?d00001