Fix favicon download from URL with non-standard port.

Fixes #5001.

The favicon download URL was constructed from scheme and host only. This is fixed by simply replacing the path of the original URL with "/favicon.ico", thus keeping scheme, host, auth and port intact.

Further modification: URL's with a non-http schema are now rejected.

Added TestIconDownloader.cpp covering corner cases.

src/gui/IconDownloader.cpp

@@ -73,18 +73,24 @@ namespace
 void IconDownloader::setUrl(const QString& entryUrl)
 {
     m_url = entryUrl;
-    QUrl url(m_url);
-    if (!url.isValid() || (url.scheme().isEmpty() && url.scheme() != "https://" && url.scheme() != "http://")) {
+    QUrl url = QUrl::fromUserInput(m_url);
+    if (!url.isValid() || url.host().isEmpty()) {
         return;
     }
 
     m_redirects = 0;
     m_urlsToTry.clear();
 
-    // If no scheme is specified, fall back to https (We don't want to compromise security, even when only downloading a
-    // favicon).
+    // If no scheme is specified, fall back to http as http is usually redirected to https, but not vice versa
     if (url.scheme().isEmpty()) {
-        url.setScheme("https://");
+        url.setScheme("http");
+    } else if (url.scheme() != "https" && url.scheme() != "http") {
+        return;
+    }
+
+    // Remove query string - if any
+    if(url.hasQuery()) {
+        url.setQuery(QString());
     }
 
     QString fullyQualifiedDomain = url.host();
@@ -101,7 +107,7 @@ void IconDownloader::setUrl(const QString& entryUrl)
     // Determine the second-level domain, if available
     QString secondLevelDomain;
     if (!hostIsIp) {
-        secondLevelDomain = getSecondLevelDomain(m_url);
+        secondLevelDomain = getSecondLevelDomain(url);
     }
 
     // Start with the "fallback" url (if enabled) to try to get the best favicon
@@ -123,7 +129,7 @@ void IconDownloader::setUrl(const QString& entryUrl)
     m_urlsToTry.append(favicon_url);
 
     // Also try a direct pull of the second-level domain (if possible)
-    if (!hostIsIp && fullyQualifiedDomain != secondLevelDomain) {
+    if (!hostIsIp && fullyQualifiedDomain != secondLevelDomain && !secondLevelDomain.isEmpty()) {
         favicon_url.setHost(secondLevelDomain);
         m_urlsToTry.append(favicon_url);
     }

src/gui/IconDownloader.h

@@ -59,6 +59,8 @@ private slots:
     QNetworkReply* m_reply;
     QTimer m_timeout;
     int m_redirects;
+
+    friend class TestIconDownloader;
 };
 
 #endif // KEEPASSXC_ICONDOWNLOADER_H

tests/CMakeLists.txt

@@ -155,6 +155,8 @@ add_unit_test(NAME testwildcardmatcher SOURCES TestWildcardMatcher.cpp
 if(WITH_XC_NETWORKING)
     add_unit_test(NAME testupdatecheck SOURCES TestUpdateCheck.cpp
             LIBS ${TEST_LIBRARIES})
+
+    add_unit_test(NAME testicondownloader SOURCES TestIconDownloader.cpp LIBS ${TEST_LIBRARIES})
 endif()
 
 if(WITH_XC_AUTOTYPE)

tests/TestIconDownloader.cpp

@@ -0,0 +1,73 @@
+#include "TestIconDownloader.h"
+#include <QTest>
+#include <gui/IconDownloader.h>
+#include <iostream>
+#include <vector>
+
+QTEST_GUILESS_MAIN(TestIconDownloader)
+
+QList<QString> TestIconDownloader::resolve_favicon_url(const char* url) {
+    IconDownloader downloader;
+    downloader.setUrl(QString(url));
+    QList<QString> resolved_urls;
+    for(const auto& resolved_url : downloader.m_urlsToTry) {
+        resolved_urls.push_back(resolved_url.toString());
+    }
+    return resolved_urls;
+}
+
+void TestIconDownloader::testFaviconResolve()
+{
+    // Create downloader
+    IconDownloader downloader;
+
+    const QString keepassxc_favicon("https://keepassxc.org/favicon.ico");
+    // Invalid URL
+    QCOMPARE(resolve_favicon_url("http:sk/s.com"), {});
+
+    // Unsupported schema
+    QCOMPARE(resolve_favicon_url("ftp://google.com"), {});
+
+    // Missing schema
+    QCOMPARE(resolve_favicon_url("keepassxc.org"), {"http://keepassxc.org/favicon.ico"});
+
+    // Missing host
+    QCOMPARE(resolve_favicon_url("https:///register"), {});
+
+    // URL with path
+    QCOMPARE(resolve_favicon_url("https://keepassxc.org/register/here"), {keepassxc_favicon});
+
+    // URL with path and query
+    QCOMPARE(resolve_favicon_url("https://keepassxc.org/register/here?login=me"), {keepassxc_favicon});
+
+    // URL with port
+    QCOMPARE(resolve_favicon_url("https://keepassxc.org:8080"), {"https://keepassxc.org:8080/favicon.ico"});
+
+    // 2nd level domain
+    QCOMPARE(resolve_favicon_url("https://login.keepassxc.org"), QList<QString>({"https://login.keepassxc.org/favicon.ico", keepassxc_favicon}));
+
+    // 2nd level domain .co.uk special case
+    QCOMPARE(resolve_favicon_url("https://login.keepassxc.co.uk"), QList<QString>({"https://login.keepassxc.co.uk/favicon.ico", "https://keepassxc.co.uk/favicon.ico"}));
+    QCOMPARE(resolve_favicon_url("https://keepassxc.co.uk"), QList<QString>({"https://keepassxc.co.uk/favicon.ico"}));
+
+    // 2nd level domain lots of subdomains
+    QCOMPARE(resolve_favicon_url("https://de.login.keepassxc.org"), QList<QString>({"https://de.login.keepassxc.org/favicon.ico", keepassxc_favicon}));
+
+    // IP
+    QCOMPARE(resolve_favicon_url("https://134.130.155.184"), QList<QString>({"https://134.130.155.184/favicon.ico"}));
+    QCOMPARE(resolve_favicon_url("134.130.155.184"), QList<QString>({"http://134.130.155.184/favicon.ico"}));
+
+    // IP with path
+    QCOMPARE(resolve_favicon_url("https://134.130.155.184/with/path"), QList<QString>({"https://134.130.155.184/favicon.ico"}));
+
+    // IP with port
+    QCOMPARE(resolve_favicon_url("https://134.130.155.184:8080/test"), QList<QString>({"https://134.130.155.184:8080/favicon.ico"}));
+    QCOMPARE(resolve_favicon_url("134.130.155.184:8080/test"), QList<QString>({"http://134.130.155.184:8080/favicon.ico"}));
+
+    // URL with Username + Password
+    QCOMPARE(resolve_favicon_url("https://user:password@keepassxc.org"), QList<QString>({"https://user:password@keepassxc.org/favicon.ico"}));
+    QCOMPARE(resolve_favicon_url("https://user:password@de.login.keepassxc.org"), QList<QString>({"https://user:password@de.login.keepassxc.org/favicon.ico", "https://user:password@keepassxc.org/favicon.ico"}));
+
+    // IP with Username + Password
+    QCOMPARE(resolve_favicon_url("https://user:password@134.130.155.184"), QList<QString>({"https://user:password@134.130.155.184/favicon.ico"}));
+}

tests/TestIconDownloader.h

@@ -0,0 +1,35 @@
+/*
+ *  Copyright (C) 2011 Felix Geyer <debfx@fobos.de>
+ *  Copyright (C) 2019 KeePassXC Team <team@keepassxc.org>
+ *
+ *  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 <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef KEEPASSXC_TESTICONDOWNLOADER_HPP
+#define KEEPASSXC_TESTICONDOWNLOADER_HPP
+
+#include <QObject>
+
+class TestIconDownloader : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void testFaviconResolve();
+
+private:
+    QList<QString> resolve_favicon_url(const char* url);
+};
+
+#endif // KEEPASSXC_TESTICONDOWNLOADER_HPP