From 9bd3ab717ebee3dbe43cf439b6acf6d6c0cb321d Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sat, 13 Feb 2016 11:13:15 +0100 Subject: [PATCH 01/15] Print libXtst instead of libXtest in the feature summary. The protocol is called XTEST but the library libxtst. Closes #440 --- src/autotype/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/autotype/CMakeLists.txt b/src/autotype/CMakeLists.txt index a0f7877fc..fb6e51277 100644 --- a/src/autotype/CMakeLists.txt +++ b/src/autotype/CMakeLists.txt @@ -2,7 +2,7 @@ if(Q_WS_X11) find_package(X11) if(PRINT_SUMMARY) add_feature_info(libXi X11_Xi_FOUND "The X11 Xi Protocol library is required for auto-type") - add_feature_info(libXtest X11_XTest_FOUND "The X11 XTEST Protocol library is required for auto-type") + add_feature_info(libXtst X11_XTest_FOUND "The X11 XTEST Protocol library is required for auto-type") endif() if(X11_FOUND AND X11_Xi_FOUND AND X11_XTest_FOUND) From 4867d26f7def490df34512ff1d3a3aaea2e291dd Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sat, 13 Feb 2016 11:24:11 +0100 Subject: [PATCH 02/15] Update optional dependencies in the README. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 326bd4a6c..a581789f5 100644 --- a/README.md +++ b/README.md @@ -57,12 +57,12 @@ The following libraries are required: * Qt 4 (>= 4.6) * libgcrypt * zlib -* libxtst (optional for auto-type on X11) +* libxi, libxtst (optional for auto-type on X11) On Debian you can install them with: ```bash -sudo apt-get install build-essential cmake libqt4-dev libgcrypt11-dev zlib1g-dev +sudo apt-get install build-essential cmake libqt4-dev libgcrypt11-dev zlib1g-dev libxi-dev libxtst-dev ``` #### Build Steps From bde4d63fdb33839d0dce691da047cd13a44cf7e6 Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sat, 9 Apr 2016 16:02:49 +0200 Subject: [PATCH 03/15] Fix typo. --- src/format/KeePass2Reader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/format/KeePass2Reader.cpp b/src/format/KeePass2Reader.cpp index d1ca9ed9c..d665bf55c 100644 --- a/src/format/KeePass2Reader.cpp +++ b/src/format/KeePass2Reader.cpp @@ -191,7 +191,7 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& ke if (!xmlReader.headerHash().isEmpty()) { QByteArray headerHash = CryptoHash::hash(headerStream.storedData(), CryptoHash::Sha256); if (headerHash != xmlReader.headerHash()) { - raiseError("Head doesn't match hash"); + raiseError("Header doesn't match hash"); return Q_NULLPTR; } } From 57c1a0f4b6ce797acf7309b20fd5214da0a9d6de Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sat, 9 Apr 2016 16:03:03 +0200 Subject: [PATCH 04/15] Show proper error message when key is wrong for .kdb files. --- src/format/KeePass1Reader.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/format/KeePass1Reader.cpp b/src/format/KeePass1Reader.cpp index 0e6b4a91c..0175a62e2 100644 --- a/src/format/KeePass1Reader.cpp +++ b/src/format/KeePass1Reader.cpp @@ -380,6 +380,10 @@ SymmetricCipherStream* KeePass1Reader::testKeys(const QString& password, const Q } } + if (!cipherStream) { + raiseError(tr("Wrong key or database file is corrupt.")); + } + return cipherStream.take(); } From 48eca3e11f99c72f72431a4c31a5c1b0eeb625f2 Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Fri, 20 May 2016 16:49:32 +0200 Subject: [PATCH 05/15] Display an error message when opening the database fails. Closes #462 --- src/gui/DatabaseOpenWidget.cpp | 3 ++- src/gui/DatabaseRepairWidget.cpp | 3 ++- src/gui/DatabaseTabWidget.cpp | 5 ++--- src/gui/KeePass1OpenWidget.cpp | 3 ++- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 7e34176dc..9647f88ed 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -99,7 +99,8 @@ void DatabaseOpenWidget::openDatabase() QFile file(m_filename); if (!file.open(QIODevice::ReadOnly)) { - // TODO: error message + MessageBox::warning(this, tr("Error"), tr("Unable to open the database.").append("\n") + .append(file.errorString())); return; } if (m_db) { diff --git a/src/gui/DatabaseRepairWidget.cpp b/src/gui/DatabaseRepairWidget.cpp index f1b760f99..e48e0f1f6 100644 --- a/src/gui/DatabaseRepairWidget.cpp +++ b/src/gui/DatabaseRepairWidget.cpp @@ -60,7 +60,8 @@ void DatabaseRepairWidget::openDatabase() QFile file(m_filename); if (!file.open(QIODevice::ReadOnly)) { - // TODO: error message + MessageBox::warning(this, tr("Error"), tr("Unable to open the database.").append("\n") + .append(file.errorString())); Q_EMIT editFinished(false); return; } diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 3d03093d7..c3b17dc61 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -131,11 +131,10 @@ void DatabaseTabWidget::openDatabase(const QString& fileName, const QString& pw, // test if we can read/write or read the file QFile file(fileName); - // TODO: error handling if (!file.open(QIODevice::ReadWrite)) { if (!file.open(QIODevice::ReadOnly)) { - // can't open - // TODO: error message + MessageBox::warning(this, tr("Error"), tr("Unable to open the database.").append("\n") + .append(file.errorString())); return; } else { diff --git a/src/gui/KeePass1OpenWidget.cpp b/src/gui/KeePass1OpenWidget.cpp index 96ddf13f2..4f70a9787 100644 --- a/src/gui/KeePass1OpenWidget.cpp +++ b/src/gui/KeePass1OpenWidget.cpp @@ -49,7 +49,8 @@ void KeePass1OpenWidget::openDatabase() QFile file(m_filename); if (!file.open(QIODevice::ReadOnly)) { - // TODO: error message + MessageBox::warning(this, tr("Error"), tr("Unable to open the database.").append("\n") + .append(file.errorString())); return; } if (m_db) { From 18e4dca6c948eac4d27da3a0f12b824b5cd950c4 Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sun, 31 Jul 2016 14:22:00 +0200 Subject: [PATCH 06/15] Explicitly include QFile in TestKeePass2Writer. Closes #452 --- tests/TestKeePass2Writer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/TestKeePass2Writer.cpp b/tests/TestKeePass2Writer.cpp index c87279454..d56d98d1d 100644 --- a/tests/TestKeePass2Writer.cpp +++ b/tests/TestKeePass2Writer.cpp @@ -18,6 +18,7 @@ #include "TestKeePass2Writer.h" #include +#include #include #include "config-keepassx-tests.h" From 8ace3ab7f275612a0fa09d750ab8a17cfd5bf306 Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sun, 31 Jul 2016 14:38:01 +0200 Subject: [PATCH 07/15] Don't consider windows with WithdrawnState as top level windows. Fixes many bogus windows in auto-type window list when using gnome-shell. --- src/autotype/x11/AutoTypeX11.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/autotype/x11/AutoTypeX11.cpp b/src/autotype/x11/AutoTypeX11.cpp index 6a20a88ff..b185ad615 100644 --- a/src/autotype/x11/AutoTypeX11.cpp +++ b/src/autotype/x11/AutoTypeX11.cpp @@ -363,13 +363,21 @@ bool AutoTypePlatformX11::isTopLevelWindow(Window window) unsigned long nitems; unsigned long after; unsigned char* data = Q_NULLPTR; - int retVal = XGetWindowProperty(m_dpy, window, m_atomWmState, 0, 0, False, AnyPropertyType, &type, &format, + int retVal = XGetWindowProperty(m_dpy, window, m_atomWmState, 0, 2, False, m_atomWmState, &type, &format, &nitems, &after, &data); - if (data) { + + bool result = false; + + if (retVal == 0 && data) { + if (type == m_atomWmState && format == 32 && nitems > 0) { + qint32 state = static_cast(*data); + result = (state != WithdrawnState); + } + XFree(data); } - return (retVal == 0) && type; + return result; } KeySym AutoTypePlatformX11::charToKeySym(const QChar& ch) From 6e2de1cd797575893ebe74a6ad037ab538ff7593 Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sun, 31 Jul 2016 14:43:33 +0200 Subject: [PATCH 08/15] Display proper error message when reading an icon fails. Refs #512 --- src/gui/EditWidgetIcons.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/gui/EditWidgetIcons.cpp b/src/gui/EditWidgetIcons.cpp index c408940e3..d8dc04f65 100644 --- a/src/gui/EditWidgetIcons.cpp +++ b/src/gui/EditWidgetIcons.cpp @@ -19,6 +19,7 @@ #include "ui_EditWidgetIcons.h" #include +#include #include "core/Group.h" #include "core/Metadata.h" @@ -129,7 +130,8 @@ void EditWidgetIcons::addCustomIcon() QString filename = QFileDialog::getOpenFileName( this, tr("Select Image"), "", filter); if (!filename.isEmpty()) { - QImage image(filename); + QImageReader imageReader(filename); + QImage image = imageReader.read(); if (!image.isNull()) { Uuid uuid = Uuid::random(); m_database->metadata()->addCustomIconScaled(uuid, image); @@ -139,7 +141,8 @@ void EditWidgetIcons::addCustomIcon() m_ui->customIconsView->setCurrentIndex(index); } else { - // TODO: show error + MessageBox::critical(this, tr("Error"), + tr("Can't read icon:").append("\n").append(imageReader.errorString())); } } } From 57ec5583966c5c1f7288b083c5385b60520c939b Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sun, 31 Jul 2016 15:36:29 +0200 Subject: [PATCH 09/15] Detect image format solely on content. Otherwise reading fails if the file extension is wrong. Closes #512 --- src/gui/EditWidgetIcons.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gui/EditWidgetIcons.cpp b/src/gui/EditWidgetIcons.cpp index d8dc04f65..6bca81c38 100644 --- a/src/gui/EditWidgetIcons.cpp +++ b/src/gui/EditWidgetIcons.cpp @@ -131,6 +131,8 @@ void EditWidgetIcons::addCustomIcon() this, tr("Select Image"), "", filter); if (!filename.isEmpty()) { QImageReader imageReader(filename); + // detect from content, otherwise reading fails if file extension is wrong + imageReader.setDecideFormatFromContent(true); QImage image = imageReader.read(); if (!image.isNull()) { Uuid uuid = Uuid::random(); From fb57ed2bcda0a9999a4221f7db5377ba7546b663 Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sun, 31 Jul 2016 17:03:25 +0200 Subject: [PATCH 10/15] Add proper error handling when QSaveFile::open() fails. Based on pull request by Valeriy Closes #450 --- src/gui/DatabaseTabWidget.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index c3b17dc61..2625dba06 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -309,6 +309,11 @@ bool DatabaseTabWidget::saveDatabase(Database* db) return false; } } + else { + MessageBox::critical(this, tr("Error"), tr("Writing the database failed.") + "\n\n" + + saveFile.errorString()); + return false; + } dbStruct.modified = false; updateTabName(db); From e9c8363b700b94218d03ac4ed2abc143089b089e Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sun, 31 Jul 2016 18:29:43 +0200 Subject: [PATCH 11/15] Save to canonical file path so we don't overwrite symlinks. When saving a database we previously replaced symlinks with a regular file. Closes #442 --- src/gui/DatabaseTabWidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 2625dba06..0478222eb 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -295,7 +295,7 @@ bool DatabaseTabWidget::saveDatabase(Database* db) DatabaseManagerStruct& dbStruct = m_dbList[db]; if (dbStruct.saveToFilename) { - QSaveFile saveFile(dbStruct.filePath); + QSaveFile saveFile(dbStruct.canonicalFilePath); if (saveFile.open(QIODevice::WriteOnly)) { m_writer.writeDatabase(&saveFile, db); if (m_writer.hasError()) { From 9532bedd7d16c65d24c3afb213f779648439416b Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Sun, 31 Jul 2016 22:07:47 +0200 Subject: [PATCH 12/15] Update min. length for password generator. Update the minimum length for the password generator depending on the chosen options. Closes #420 --- src/gui/PasswordGeneratorWidget.cpp | 38 +++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/gui/PasswordGeneratorWidget.cpp b/src/gui/PasswordGeneratorWidget.cpp index d6fb12bea..4cb14c4de 100644 --- a/src/gui/PasswordGeneratorWidget.cpp +++ b/src/gui/PasswordGeneratorWidget.cpp @@ -111,6 +111,10 @@ void PasswordGeneratorWidget::sliderMoved() void PasswordGeneratorWidget::spinBoxChanged() { + if (m_updatingSpinBox) { + return; + } + // Interlock so that we don't update twice - this causes issues as the spinbox can go higher than slider m_updatingSpinBox = true; @@ -161,9 +165,39 @@ PasswordGenerator::GeneratorFlags PasswordGeneratorWidget::generatorFlags() void PasswordGeneratorWidget::updateGenerator() { + PasswordGenerator::CharClasses classes = charClasses(); + PasswordGenerator::GeneratorFlags flags = generatorFlags(); + + int minLength = 0; + if (flags.testFlag(PasswordGenerator::CharFromEveryGroup)) { + if (classes.testFlag(PasswordGenerator::LowerLetters)) { + minLength++; + } + if (classes.testFlag(PasswordGenerator::UpperLetters)) { + minLength++; + } + if (classes.testFlag(PasswordGenerator::Numbers)) { + minLength++; + } + if (classes.testFlag(PasswordGenerator::SpecialCharacters)) { + minLength++; + } + } + minLength = qMax(minLength, 1); + + if (m_ui->spinBoxLength->value() < minLength) { + m_updatingSpinBox = true; + m_ui->spinBoxLength->setValue(minLength); + m_ui->sliderLength->setValue(minLength); + m_updatingSpinBox = false; + } + + m_ui->spinBoxLength->setMinimum(minLength); + m_ui->sliderLength->setMinimum(minLength); + m_generator->setLength(m_ui->spinBoxLength->value()); - m_generator->setCharClasses(charClasses()); - m_generator->setFlags(generatorFlags()); + m_generator->setCharClasses(classes); + m_generator->setFlags(flags); if (m_generator->isValid()) { QString password = m_generator->generatePassword(); From bf2fd631315cd1cf621ed34c3ce1a880b0b215e0 Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Wed, 3 Aug 2016 23:24:08 +0200 Subject: [PATCH 13/15] Fix crash when deleting parent group of recycle bin. In these cases delete the group instead of trying to move it to the recycle bin. Closes #520 --- src/gui/DatabaseWidget.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index 8de842f3a..7191c8e28 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -493,7 +493,9 @@ void DatabaseWidget::deleteGroup() } bool inRecylceBin = Tools::hasChild(m_db->metadata()->recycleBin(), currentGroup); - if (inRecylceBin || !m_db->metadata()->recycleBinEnabled()) { + bool isRecycleBin = (currentGroup == m_db->metadata()->recycleBin()); + bool isRecycleBinSubgroup = Tools::hasChild(currentGroup, m_db->metadata()->recycleBin()); + if (inRecylceBin || isRecycleBin || isRecycleBinSubgroup || !m_db->metadata()->recycleBinEnabled()) { QMessageBox::StandardButton result = MessageBox::question( this, tr("Delete group?"), tr("Do you really want to delete the group \"%1\" for good?") From cd1192b409399f97ce71f43f8011434ffd37f0a2 Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Wed, 3 Aug 2016 23:45:04 +0200 Subject: [PATCH 14/15] Allow deleting the recycle bin. Closes #46 --- src/gui/DatabaseWidget.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index 7191c8e28..92720e660 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -873,8 +873,7 @@ bool DatabaseWidget::dbHasKey() const bool DatabaseWidget::canDeleteCurrentGroup() const { bool isRootGroup = m_db->rootGroup() == m_groupView->currentGroup(); - bool isRecycleBin = m_db->metadata()->recycleBin() == m_groupView->currentGroup(); - return !isRootGroup && !isRecycleBin; + return !isRootGroup; } bool DatabaseWidget::isInSearchMode() const From 8d16522d393e42b440af3b963a7ea202e28dc535 Mon Sep 17 00:00:00 2001 From: Florian Geyer Date: Wed, 3 Aug 2016 00:00:56 +0200 Subject: [PATCH 15/15] Repair UUID of inconsistent history items. Closes #130 --- src/core/Entry.cpp | 1 - src/format/KeePass2XmlReader.cpp | 7 +++++ tests/TestKeePass2XmlReader.cpp | 28 +++++++++++++++++++ tests/TestKeePass2XmlReader.h | 1 + .../data/BrokenDifferentEntryHistoryUuid.xml | 17 +++++++++++ 5 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tests/data/BrokenDifferentEntryHistoryUuid.xml diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 784b489cd..1bc89e990 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -372,7 +372,6 @@ const QList& Entry::historyItems() const void Entry::addHistoryItem(Entry* entry) { Q_ASSERT(!entry->parent()); - Q_ASSERT(entry->uuid() == uuid()); m_history.append(entry); Q_EMIT modified(); diff --git a/src/format/KeePass2XmlReader.cpp b/src/format/KeePass2XmlReader.cpp index 815cbf9c2..2e76c2a90 100644 --- a/src/format/KeePass2XmlReader.cpp +++ b/src/format/KeePass2XmlReader.cpp @@ -778,6 +778,13 @@ Entry* KeePass2XmlReader::parseEntry(bool history) } Q_FOREACH (Entry* historyItem, historyItems) { + if (historyItem->uuid() != entry->uuid()) { + if (m_strictMode) { + raiseError("History element with different uuid"); + } else { + historyItem->setUuid(entry->uuid()); + } + } entry->addHistoryItem(historyItem); } diff --git a/tests/TestKeePass2XmlReader.cpp b/tests/TestKeePass2XmlReader.cpp index dd8132708..014587f48 100644 --- a/tests/TestKeePass2XmlReader.cpp +++ b/tests/TestKeePass2XmlReader.cpp @@ -407,6 +407,8 @@ void TestKeePass2XmlReader::testBroken_data() QTest::newRow("BrokenGroupReference (not strict)") << "BrokenGroupReference" << false << false; QTest::newRow("BrokenDeletedObjects (strict)") << "BrokenDeletedObjects" << true << true; QTest::newRow("BrokenDeletedObjects (not strict)") << "BrokenDeletedObjects" << false << false; + QTest::newRow("BrokenDifferentEntryHistoryUuid (strict)") << "BrokenDifferentEntryHistoryUuid" << true << true; + QTest::newRow("BrokenDifferentEntryHistoryUuid (not strict)") << "BrokenDifferentEntryHistoryUuid" << false << false; } void TestKeePass2XmlReader::testEmptyUuids() @@ -487,6 +489,32 @@ void TestKeePass2XmlReader::testInvalidXmlChars() QCOMPARE(strToBytes(attrRead->value("SurrogateValid2")), strToBytes(strSurrogateValid2)); } +void TestKeePass2XmlReader::testRepairUuidHistoryItem() +{ + KeePass2XmlReader reader; + QString xmlFile = QString("%1/%2.xml").arg(KEEPASSX_TEST_DATA_DIR, "BrokenDifferentEntryHistoryUuid"); + QVERIFY(QFile::exists(xmlFile)); + QScopedPointer db(reader.readDatabase(xmlFile)); + if (reader.hasError()) { + qWarning("Database read error: %s", qPrintable(reader.errorString())); + } + QVERIFY(!reader.hasError()); + + + + QList entries = db.data()->rootGroup()->entries(); + QCOMPARE(entries.size(), 1); + Entry* entry = entries.at(0); + + QList historyItems = entry->historyItems(); + QCOMPARE(historyItems.size(), 1); + Entry* historyItem = historyItems.at(0); + + QVERIFY(!entry->uuid().isNull()); + QVERIFY(!historyItem->uuid().isNull()); + QCOMPARE(historyItem->uuid(), entry->uuid()); +} + void TestKeePass2XmlReader::cleanupTestCase() { delete m_db; diff --git a/tests/TestKeePass2XmlReader.h b/tests/TestKeePass2XmlReader.h index b9be7b553..ff83e2597 100644 --- a/tests/TestKeePass2XmlReader.h +++ b/tests/TestKeePass2XmlReader.h @@ -43,6 +43,7 @@ private Q_SLOTS: void testBroken_data(); void testEmptyUuids(); void testInvalidXmlChars(); + void testRepairUuidHistoryItem(); void cleanupTestCase(); private: diff --git a/tests/data/BrokenDifferentEntryHistoryUuid.xml b/tests/data/BrokenDifferentEntryHistoryUuid.xml new file mode 100644 index 000000000..c07f03aa7 --- /dev/null +++ b/tests/data/BrokenDifferentEntryHistoryUuid.xml @@ -0,0 +1,17 @@ + + + + + lmU+9n0aeESKZvcEze+bRg== + Test + + MTExMTExMTExMTExMTExMQ== + + + MjIyMjIyMjIyMjIyMjIyMg== + + + + + +