From 013f3cea70acfe7b701cb73c93744d5ff5c0c213 Fri Feb 5 10:06:25 2021 From: allexzander Date: Fri, 5 Feb 2021 10:06:25 +0200 Subject: [PATCH] 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 | 11 +++++++++++ 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(+), 15 deletions(-) --- a/src/gui/accountsettings.cpp +++ b/src/gui/accountsettings.cpp @@ -36,6 +36,7 @@ #include "encryptfolderjob.h" #include "syncresult.h" #include "ignorelisttablewidget.h" +#include "guiutility.h" #include @@ -705,8 +706,9 @@ void AccountSettings::slotForceSyncCurre void AccountSettings::slotOpenOC() { - if (_OCUrl.isValid()) - QDesktopServices::openUrl(_OCUrl); + if (_OCUrl.isValid()) { + Utility::openBrowser(_OCUrl); + } } void AccountSettings::slotUpdateQuota(qint64 total, qint64 used) --- 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 Toke { 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); --- a/src/gui/creds/oauth.cpp +++ b/src/gui/creds/oauth.cpp @@ -22,6 +22,7 @@ #include #include "theme.h" #include "networkjobs.h" +#include "guiutility.h" namespace OCC { @@ -165,7 +166,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; --- a/src/gui/guiutility.cpp +++ b/src/gui/guiutility.cpp @@ -27,6 +27,17 @@ Q_LOGGING_CATEGORY(lcUtility, "nextcloud 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( --- 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 @@ -570,7 +571,7 @@ void ownCloudGui::slotToggleLogBrowser() void ownCloudGui::slotOpenOwnCloud() { if (auto account = qvariant_cast(sender()->property(propertyAccountC))) { - QDesktopServices::openUrl(account->url()); + Utility::openBrowser(account->url()); } } --- a/src/gui/socketapi.cpp +++ b/src/gui/socketapi.cpp @@ -499,7 +499,7 @@ void SocketApi::command_EDIT(const QStri auto url = QUrl(data.value("url").toString()); if(!url.isEmpty()) - Utility::openBrowser(url, nullptr); + Utility::openBrowser(url); }); job->start(); } @@ -772,7 +772,7 @@ void SocketApi::emailPrivateLink(const Q void OCC::SocketApi::openPrivateLink(const QString &link) { - Utility::openBrowser(link, nullptr); + Utility::openBrowser(link); } void SocketApi::command_GET_STRINGS(const QString &argument, SocketListener *listener) --- a/src/gui/tray/ActivityListModel.cpp +++ b/src/gui/tray/ActivityListModel.cpp @@ -26,6 +26,7 @@ #include "folderman.h" #include "iconjob.h" #include "accessmanager.h" +#include "guiutility.h" #include "ActivityData.h" #include "ActivityListModel.h" @@ -458,7 +459,7 @@ void ActivityListModel::triggerDefaultAc QDesktopServices::openUrl(path); } else { const auto link = data(modelIndex, LinkRole).toUrl(); - QDesktopServices::openUrl(link); + Utility::openBrowser(link); } } @@ -479,7 +480,7 @@ void ActivityListModel::triggerAction(in const auto action = activity._links[actionIndex]; if (action._verb == "WEB") { - QDesktopServices::openUrl(QUrl(action._link)); + Utility::openBrowser(QUrl(action._link)); return; } --- a/src/gui/tray/UserModel.cpp +++ b/src/gui/tray/UserModel.cpp @@ -8,6 +8,7 @@ #include "configfile.h" #include "notificationconfirmjob.h" #include "logger.h" +#include "guiutility.h" #include #include @@ -647,7 +648,7 @@ Q_INVOKABLE void UserModel::openCurrentA 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(); } @@ -659,10 +660,11 @@ Q_INVOKABLE void UserModel::openCurrentA 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) @@ -911,7 +913,7 @@ void UserAppsModel::buildAppList() void UserAppsModel::openAppUrl(const QUrl &url) { - QDesktopServices::openUrl(url); + Utility::openBrowser(url); } int UserAppsModel::rowCount(const QModelIndex &parent) const --- 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::slotOpenS { Theme *theme = Theme::instance(); QUrl url = QUrl(field("OCUrl").toString() + theme->wizardUrlPostfix()); - QDesktopServices::openUrl(url); + Utility::openBrowser(url); } } // namespace OCC --- 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::acceptNaviga { Q_UNUSED(type); Q_UNUSED(isMainFrame); - QDesktopServices::openUrl(url); + Utility::openBrowser(url); return false; } --- a/src/gui/guiutility.h +++ b/src/gui/guiutility.h @@ -26,7 +26,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. * --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -96,7 +96,7 @@ list(APPEND RemoteWipe_SRC ${RemoteWipe_ list(APPEND RemoteWipe_SRC stubremotewipe.cpp ) nextcloud_add_test(RemoteWipe "${RemoteWipe_SRC}") -nextcloud_add_test(OAuth "syncenginetestutils.h;../src/gui/creds/oauth.cpp") +nextcloud_add_test(OAuth "syncenginetestutils.h;../src/gui/creds/oauth.cpp;../src/gui/guiutility.cpp") configure_file(test_journal.db "${PROJECT_BINARY_DIR}/bin/test_journal.db" COPYONLY)