Required fixes

keepassxreboot · Oct 10, 2021 · 01d2e05 · 01d2e05
1 parent 889f507
commit 01d2e05
Show file tree

Hide file tree

Showing 10 changed files with 44 additions and 6 deletions.
diff --git a/src/autotype/AutoType.cpp b/src/autotype/AutoType.cpp
@@ -170,9 +170,9 @@ void AutoType::loadPlugin(const QString& pluginPath)
                 connect(osUtils,
                         &OSUtilsBase::globalShortcutTriggered,
                         this,
-                        [this](const QString& name, const QString& url) {
+                        [this](const QString& name, const QString& initialSearch) {
                             if (name == "autotype") {
-                                startGlobalAutoType(url);
+                                startGlobalAutoType(initialSearch);
                             }
                         });
             } else {

diff --git a/src/autotype/AutoType.h b/src/autotype/AutoType.h
@@ -55,7 +55,7 @@ class AutoType : public QObject
     static void createTestInstance();
 
 public slots:
-    void performGlobalAutoType(const QList<QSharedPointer<Database>>& dbList, const QString& search = QString());
+    void performGlobalAutoType(const QList<QSharedPointer<Database>>& dbList, const QString& search = {});
     void raiseWindow();
 
 signals:

diff --git a/src/browser/BrowserAction.cpp b/src/browser/BrowserAction.cpp
@@ -28,6 +28,8 @@
 #include <sodium/crypto_box.h>
 #include <sodium/randombytes.h>
 
+const int BrowserAction::MaxUrlLength = 256;
+
 namespace
 {
     enum
@@ -517,6 +519,10 @@ QJsonObject BrowserAction::handleGlobalAutoType(const QJsonObject& json, const Q
     }
 
     const auto topLevelDomain = decrypted.value("search").toString();
+    if (topLevelDomain.length() > BrowserAction::MaxUrlLength) {
+        return getErrorReply(action, ERROR_KEEPASS_NO_URL_PROVIDED);
+    }
+
     browserService()->requestGlobalAutoType(topLevelDomain);
 
     const QString newNonce = incrementNonce(nonce);

diff --git a/src/browser/BrowserAction.h b/src/browser/BrowserAction.h
@@ -62,6 +62,8 @@ class BrowserAction
     QString incrementNonce(const QString& nonce);
 
 private:
+    static const int MaxUrlLength;
+
     QString m_clientPublicKey;
     QString m_publicKey;
     QString m_secretKey;

diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp
@@ -34,6 +34,7 @@
 #include "gui/MessageBox.h"
 #include "gui/osutils/OSUtils.h"
 #include <QCheckBox>
+#include <QHostAddress>
 #include <QInputDialog>
 #include <QJsonArray>
 #include <QMessageBox>
@@ -995,6 +996,12 @@ bool BrowserService::schemeFound(const QString& url)
     return !address.scheme().isEmpty();
 }
 
+bool BrowserService::isIpAddress(const QString& host) const
+{
+    QHostAddress address(host);
+    return address.protocol() == QAbstractSocket::IPv4Protocol || address.protocol() == QAbstractSocket::IPv6Protocol;
+}
+
 bool BrowserService::removeFirstDomain(QString& hostname)
 {
     int pos = hostname.indexOf(".");
@@ -1092,8 +1099,7 @@ QString BrowserService::getTopLevelDomainFromUrl(const QString& url) const
     QString host = qurl.host();
 
     // If the hostname is an IP address, return it directly
-    QRegularExpression re("^[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}$");
-    if (re.match(host).hasMatch()) {
+    if (isIpAddress(host)) {
         return host;
     }
 

diff --git a/src/browser/BrowserService.h b/src/browser/BrowserService.h
@@ -136,6 +136,7 @@ private slots:
     Group* getDefaultEntryGroup(const QSharedPointer<Database>& selectedDb = {});
     int sortPriority(const QStringList& urls, const QString& siteUrlStr, const QString& formUrlStr);
     bool schemeFound(const QString& url);
+    bool isIpAddress(const QString& host) const;
     bool removeFirstDomain(QString& hostname);
     bool handleEntry(Entry* entry, const QString& url, const QString& submitUrl);
     bool handleURL(const QString& entryUrl, const QString& siteUrlStr, const QString& formUrlStr);

diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp
@@ -1187,6 +1187,7 @@ void DatabaseWidget::unlockDatabase(bool accepted)
         QList<QSharedPointer<Database>> dbList;
         dbList.append(m_db);
         autoType()->performGlobalAutoType(dbList, m_searchStringForAutoType);
+        m_searchStringForAutoType.clear();
     }
 }
 

diff --git a/src/gui/osutils/OSUtilsBase.h b/src/gui/osutils/OSUtilsBase.h
@@ -69,7 +69,7 @@ class OSUtilsBase : public QObject
     virtual bool setPreventScreenCapture(QWindow* window, bool allow) const;
 
 signals:
-    void globalShortcutTriggered(const QString& name, const QString& search = QString());
+    void globalShortcutTriggered(const QString& name, const QString& search = {});
 
     /**
      * Indicates platform UI theme change (light mode to dark mode).

diff --git a/tests/TestBrowser.cpp b/tests/TestBrowser.cpp
@@ -669,3 +669,24 @@ void TestBrowser::testBestMatchingWithAdditionalURLs()
     QCOMPARE(sorted.length(), 1);
     QCOMPARE(sorted[0]->url(), urls[0]);
 }
+
+void TestBrowser::testIsIpAddress()
+{
+    auto host1 = "example.com"; // Not valid
+    auto host2 = "192.168.0.1";
+    auto host3 = "278.21.2.0"; // Not valid
+    auto host4 = "2001:0db8:85a3:0000:0000:8a2e:0370:7334";
+    auto host5 = "2001:db8:0:1:1:1:1:1";
+    auto host6 = "fe80::1ff:fe23:4567:890a";
+    auto host7 = "2001:20::1";
+    auto host8 = "2001:0db8:85y3:0000:0000:8a2e:0370:7334"; // Not valid
+
+    QVERIFY(!m_browserService->isIpAddress(host1));
+    QVERIFY(m_browserService->isIpAddress(host2));
+    QVERIFY(!m_browserService->isIpAddress(host3));
+    QVERIFY(m_browserService->isIpAddress(host4));
+    QVERIFY(m_browserService->isIpAddress(host5));
+    QVERIFY(m_browserService->isIpAddress(host6));
+    QVERIFY(m_browserService->isIpAddress(host7));
+    QVERIFY(!m_browserService->isIpAddress(host8));
+}
diff --git a/tests/TestBrowser.h b/tests/TestBrowser.h
@@ -51,6 +51,7 @@ private slots:
     void testValidURLs();
     void testBestMatchingCredentials();
     void testBestMatchingWithAdditionalURLs();
+    void testIsIpAddress();
 
 private:
     QList<Entry*> createEntries(QStringList& urls, Group* root) const;