From e97b7d8a25d3ef0d8c52b6399f304a42a5d4f212 Mon Sep 17 00:00:00 2001 From: allexzander Date: Mon, 8 Feb 2021 11:50:14 +0200 Subject: [PATCH 1/2] Default parameter nullptr widget for openBrowser. Signed-off-by: allexzander --- src/gui/guiutility.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/guiutility.h b/src/gui/guiutility.h index 6e2c5f1635345..ece8b3bd7b6c5 100644 --- a/src/gui/guiutility.h +++ b/src/gui/guiutility.h @@ -28,7 +28,7 @@ namespace Utility { * * If launching the browser fails, display a message. */ - bool openBrowser(const QUrl &url, QWidget *errorWidgetParent); + bool openBrowser(const QUrl &url, QWidget *errorWidgetParent = nullptr); /** Start composing a new email message. * From 013f3cea70acfe7b701cb73c93744d5ff5c0c213 Mon Sep 17 00:00:00 2001 From: allexzander Date: Fri, 5 Feb 2021 10:06:25 +0200 Subject: [PATCH 2/2] Validate sensitive URLs to onle allow http(s) schemes. Signed-off-by: allexzander --- src/gui/accountsettings.cpp | 5 +++-- src/gui/creds/flow2auth.cpp | 3 ++- src/gui/creds/oauth.cpp | 3 ++- src/gui/guiutility.cpp | 12 +++++++++++- src/gui/owncloudgui.cpp | 3 ++- src/gui/socketapi.cpp | 4 ++-- src/gui/tray/ActivityListModel.cpp | 5 +++-- src/gui/tray/UserModel.cpp | 10 ++++++---- src/gui/wizard/owncloudwizardresultpage.cpp | 3 ++- src/gui/wizard/webview.cpp | 3 ++- 10 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index 5b340de407311..d3a2b955f7f2f 100644 --- 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 index 25f215a0a3ade..f8281ea4dd9cc 100644 --- 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 index a22b15bdc2d7c..31771d388d5cd 100644 --- 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 index 427ebb4b20f86..9a86075b805bf 100644 --- a/src/gui/guiutility.cpp +++ b/src/gui/guiutility.cpp @@ -22,13 +22,23 @@ #include #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 index 78c4efcab4460..2ea6d9052923e 100644 --- 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(sender()->property(propertyAccountC))) { - QDesktopServices::openUrl(account->url()); + Utility::openBrowser(account->url()); } } diff --git a/src/gui/socketapi.cpp b/src/gui/socketapi.cpp index b1b4e7eaa7efa..7f821a6450154 100644 --- 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 index 09b77065f8404..98bda199b5cae 100644 --- 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 index cafdcbe489faf..a175d3173b45e 100644 --- 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 #include @@ -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 index 826ef0ade756c..d3b1e72680576 100644 --- a/src/gui/wizard/owncloudwizardresultpage.cpp +++ b/src/gui/wizard/owncloudwizardresultpage.cpp @@ -17,6 +17,7 @@ #include #include +#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 index d8d36086bc5fb..e03f8650965c0 100644 --- a/src/gui/wizard/webview.cpp +++ b/src/gui/wizard/webview.cpp @@ -16,6 +16,7 @@ #include #include +#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; }