Validate sensitive URLs to onle allow http(s) schemes.

Signed-off-by: allexzander <blackslayer4@gmail.com>
nextcloud · Feb 9, 2021 · 013f3ce · 013f3ce
1 parent e97b7d8
commit 013f3ce
Show file tree

Hide file tree

Showing 10 changed files with 35 additions and 16 deletions.
diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp
@@ -1017,8 +1017,9 @@ void AccountSettings::slotForceSyncCurrentFolder()
 
 void AccountSettings::slotOpenOC()
 {
-    if (_OCUrl.isValid())
-        QDesktopServices::openUrl(_OCUrl);
+    if (_OCUrl.isValid()) {
+        Utility::openBrowser(_OCUrl);
+    }
 }
 
 void AccountSettings::slotUpdateQuota(qint64 total, qint64 used)

diff --git a/src/gui/creds/flow2auth.cpp b/src/gui/creds/flow2auth.cpp
@@ -25,6 +25,7 @@
 #include "theme.h"
 #include "networkjobs.h"
 #include "configfile.h"
+#include "guiutility.h"
 
 namespace OCC {
 
@@ -146,7 +147,7 @@ void Flow2Auth::fetchNewToken(const TokenAction action)
         {
         case actionOpenBrowser:
             // Try to open Browser
-            if (!QDesktopServices::openUrl(authorisationLink())) {
+            if (!Utility::openBrowser(authorisationLink())) {
                 // We cannot open the browser, then we claim we don't support Flow2Auth.
                 // Our UI callee will ask the user to copy and open the link.
                 emit result(NotSupported);

diff --git a/src/gui/creds/oauth.cpp b/src/gui/creds/oauth.cpp
@@ -23,6 +23,7 @@
 #include "theme.h"
 #include "networkjobs.h"
 #include "creds/httpcredentials.h"
+#include "guiutility.h"
 
 namespace OCC {
 
@@ -174,7 +175,7 @@ QUrl OAuth::authorisationLink() const
 
 bool OAuth::openBrowser()
 {
-    if (!QDesktopServices::openUrl(authorisationLink())) {
+    if (!Utility::openBrowser(authorisationLink())) {
         // We cannot open the browser, then we claim we don't support OAuth.
         emit result(NotSupported, QString());
         return false;

diff --git a/src/gui/guiutility.cpp b/src/gui/guiutility.cpp
@@ -22,13 +22,23 @@
 #include <QUrlQuery>
 
 #include "common/asserts.h"
-
 using namespace OCC;
 
 Q_LOGGING_CATEGORY(lcUtility, "nextcloud.gui.utility", QtInfoMsg)
 
 bool Utility::openBrowser(const QUrl &url, QWidget *errorWidgetParent)
 {
+    const QStringList allowedUrlSchemes = {
+        "http",
+        "https",
+        "oauthtest"
+    };
+
+    if (!allowedUrlSchemes.contains(url.scheme())) {
+        qCWarning(lcUtility) << "URL format is not supported, or it has been compromised for:" << url.toString();
+        return false;
+    }
+
     if (!QDesktopServices::openUrl(url)) {
         if (errorWidgetParent) {
             QMessageBox::warning(

diff --git a/src/gui/owncloudgui.cpp b/src/gui/owncloudgui.cpp
@@ -28,6 +28,7 @@
 #include "accountmanager.h"
 #include "common/syncjournalfilerecord.h"
 #include "creds/abstractcredentials.h"
+#include "guiutility.h"
 #ifdef WITH_LIBCLOUDPROVIDERS
 #include "cloudproviders/cloudprovidermanager.h"
 #endif
@@ -573,7 +574,7 @@ void ownCloudGui::slotToggleLogBrowser()
 void ownCloudGui::slotOpenOwnCloud()
 {
     if (auto account = qvariant_cast<AccountPtr>(sender()->property(propertyAccountC))) {
-        QDesktopServices::openUrl(account->url());
+        Utility::openBrowser(account->url());
     }
 }
 

diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp
@@ -597,7 +597,7 @@ void SocketApi::command_EDIT(const QString &localFile, SocketListener *listener)
         auto url = QUrl(data.value("url").toString());
 
         if(!url.isEmpty())
-            Utility::openBrowser(url, nullptr);
+            Utility::openBrowser(url);
     });
     job->start();
 }
@@ -907,7 +907,7 @@ void SocketApi::emailPrivateLink(const QString &link)
 
 void OCC::SocketApi::openPrivateLink(const QString &link)
 {
-    Utility::openBrowser(link, nullptr);
+    Utility::openBrowser(link);
 }
 
 void SocketApi::command_GET_STRINGS(const QString &argument, SocketListener *listener)

diff --git a/src/gui/tray/ActivityListModel.cpp b/src/gui/tray/ActivityListModel.cpp
@@ -27,6 +27,7 @@
 #include "iconjob.h"
 #include "accessmanager.h"
 #include "owncloudgui.h"
+#include "guiutility.h"
 
 #include "ActivityData.h"
 #include "ActivityListModel.h"
@@ -465,7 +466,7 @@ void ActivityListModel::triggerDefaultAction(int activityIndex)
         QDesktopServices::openUrl(path);
     } else {
         const auto link = data(modelIndex, LinkRole).toUrl();
-        QDesktopServices::openUrl(link);
+        Utility::openBrowser(link);
     }
 }
 
@@ -486,7 +487,7 @@ void ActivityListModel::triggerAction(int activityIndex, int actionIndex)
     const auto action = activity._links[actionIndex];
 
     if (action._verb == "WEB") {
-        QDesktopServices::openUrl(QUrl(action._link));
+        Utility::openBrowser(QUrl(action._link));
         return;
     }
 

diff --git a/src/gui/tray/UserModel.cpp b/src/gui/tray/UserModel.cpp
@@ -9,6 +9,7 @@
 #include "configfile.h"
 #include "notificationconfirmjob.h"
 #include "logger.h"
+#include "guiutility.h"
 
 #include <QDesktopServices>
 #include <QIcon>
@@ -719,7 +720,7 @@ Q_INVOKABLE void UserModel::openCurrentAccountTalk()
 
     const auto talkApp = currentUser()->talkApp();
     if (talkApp) {
-        QDesktopServices::openUrl(talkApp->url());
+        Utility::openBrowser(talkApp->url());
     } else {
         qCWarning(lcActivity) << "The Talk app is not enabled on" << currentUser()->server();
     }
@@ -731,10 +732,11 @@ Q_INVOKABLE void UserModel::openCurrentAccountServer()
         return;
 
     QString url = _users[_currentUserId]->server(false);
-    if (!(url.contains("http://") || url.contains("https://"))) {
+    if (!url.startsWith("http://") && !url.startsWith("https://")) {
         url = "https://" + _users[_currentUserId]->server(false);
     }
-    QDesktopServices::openUrl(QUrl(url));
+
+    QDesktopServices::openUrl(url);
 }
 
 Q_INVOKABLE void UserModel::switchCurrentUser(const int &id)
@@ -983,7 +985,7 @@ void UserAppsModel::buildAppList()
 
 void UserAppsModel::openAppUrl(const QUrl &url)
 {
-    QDesktopServices::openUrl(url);
+    Utility::openBrowser(url);
 }
 
 int UserAppsModel::rowCount(const QModelIndex &parent) const

diff --git a/src/gui/wizard/owncloudwizardresultpage.cpp b/src/gui/wizard/owncloudwizardresultpage.cpp
@@ -17,6 +17,7 @@
 #include <QDir>
 #include <QUrl>
 
+#include "guiutility.h"
 #include "wizard/owncloudwizardresultpage.h"
 #include "wizard/owncloudwizardcommon.h"
 #include "theme.h"
@@ -93,7 +94,7 @@ void OwncloudWizardResultPage::slotOpenServer()
 {
     Theme *theme = Theme::instance();
     QUrl url = QUrl(field("OCUrl").toString() + theme->wizardUrlPostfix());
-    QDesktopServices::openUrl(url);
+    Utility::openBrowser(url);
 }
 
 } // namespace OCC
diff --git a/src/gui/wizard/webview.cpp b/src/gui/wizard/webview.cpp
@@ -16,6 +16,7 @@
 #include <QWebEngineCertificateError>
 #include <QMessageBox>
 
+#include "guiutility.h"
 #include "common/utility.h"
 
 namespace OCC {
@@ -227,7 +228,7 @@ bool ExternalWebEnginePage::acceptNavigationRequest(const QUrl &url, QWebEngineP
 {
     Q_UNUSED(type);
     Q_UNUSED(isMainFrame);
-    QDesktopServices::openUrl(url);
+    Utility::openBrowser(url);
     return false;
 }