Merge branch '2.0'

README.md

@@ -57,12 +57,12 @@ The following libraries are required:
 * Qt 5 (>= 5.2): qtbase and qttools5
 * libgcrypt (>= 1.6)
 * zlib
-* libxtst, qtx11extras (optional for auto-type on X11)
+* libxi, libxtst, qtx11extras (optional for auto-type on X11)
 
 On Debian you can install them with:
 
 ```bash
-sudo apt-get install build-essential cmake qtbase5-dev libqt5x11extras5-dev qttools5-dev qttools5-dev-tools libgcrypt20-dev zlib1g-dev
+sudo apt-get install build-essential cmake qtbase5-dev libqt5x11extras5-dev qttools5-dev qttools5-dev-tools libgcrypt20-dev zlib1g-dev libxi-dev libxtst-dev
 ```
 
 #### Build Steps

src/autotype/CMakeLists.txt

@@ -2,7 +2,7 @@ if(UNIX AND NOT APPLE)
   find_package(X11)
   find_package(Qt5X11Extras 5.2)
   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")
   add_feature_info(Qt5X11Extras Qt5X11Extras_FOUND "The Qt5X11Extras library is required for auto-type")
 
   if(X11_FOUND AND X11_Xi_FOUND AND X11_XTest_FOUND AND Qt5X11Extras_FOUND)

src/autotype/xcb/AutoTypeXCB.cpp

@@ -385,13 +385,21 @@ bool AutoTypePlatformX11::isTopLevelWindow(Window window)
     unsigned long nitems;
     unsigned long after;
     unsigned char* data = 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<qint32>(*data);
+            result = (state != WithdrawnState);
+        }
+
         XFree(data);
     }
 
-    return (retVal == 0) && type;
+    return result;
 }
 
 KeySym AutoTypePlatformX11::charToKeySym(const QChar& ch)

src/core/Entry.cpp

@@ -371,7 +371,6 @@ const QList<Entry*>& Entry::historyItems() const
 void Entry::addHistoryItem(Entry* entry)
 {
     Q_ASSERT(!entry->parent());
-    Q_ASSERT(entry->uuid() == uuid());
 
     m_history.append(entry);
     Q_EMIT modified();

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();
 }
 

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 nullptr;
         }
     }

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);
     }
 

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) {

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;
     }

src/gui/DatabaseTabWidget.cpp

@@ -132,11 +132,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 {
@@ -297,7 +296,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()) {
@@ -311,6 +310,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);

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?")
@@ -871,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

src/gui/EditWidgetIcons.cpp

@@ -19,6 +19,7 @@
 #include "ui_EditWidgetIcons.h"
 
 #include <QFileDialog>
+#include <QImageReader>
 
 #include "core/Group.h"
 #include "core/Metadata.h"
@@ -129,7 +130,10 @@ void EditWidgetIcons::addCustomIcon()
         QString filename = QFileDialog::getOpenFileName(
                     this, tr("Select Image"), "", filter);
         if (!filename.isEmpty()) {
-            QImage image(filename);
+            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();
                 m_database->metadata()->addCustomIconScaled(uuid, image);
@@ -139,7 +143,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()));
             }
         }
     }

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) {

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();

tests/TestKeePass2Writer.cpp

@@ -18,6 +18,7 @@
 #include "TestKeePass2Writer.h"
 
 #include <QBuffer>
+#include <QFile>
 #include <QTest>
 
 #include "config-keepassx-tests.h"

tests/TestKeePass2XmlReader.cpp

@@ -406,6 +406,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()
@@ -486,6 +488,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<Database> db(reader.readDatabase(xmlFile));
+    if (reader.hasError()) {
+        qWarning("Database read error: %s", qPrintable(reader.errorString()));
+    }
+    QVERIFY(!reader.hasError());
+
+
+
+    QList<Entry*> entries = db.data()->rootGroup()->entries();
+    QCOMPARE(entries.size(), 1);
+    Entry* entry = entries.at(0);
+
+    QList<Entry*> 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;

tests/TestKeePass2XmlReader.h

@@ -43,6 +43,7 @@ private Q_SLOTS:
     void testBroken_data();
     void testEmptyUuids();
     void testInvalidXmlChars();
+    void testRepairUuidHistoryItem();
     void cleanupTestCase();
 
 private:

tests/data/BrokenDifferentEntryHistoryUuid.xml

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8" standalone="yes"?>
+<KeePassFile>
+	<Root>
+		<Group>
+			<UUID>lmU+9n0aeESKZvcEze+bRg==</UUID>
+			<Name>Test</Name>
+			<Entry>
+				<UUID>MTExMTExMTExMTExMTExMQ==</UUID>
+				<History>
+					<Entry>
+						<UUID>MjIyMjIyMjIyMjIyMjIyMg==</UUID>
+					</Entry>
+				</History>
+			</Entry>
+		</Group>
+	</Root>
+</KeePassFile>