From 31b3f0e8f1faa8f9e4a2d89c2375607deaed2fb3 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Tue, 23 Apr 2019 17:34:46 -0700 Subject: [PATCH] possible fix for openUrl exploit --- .../qml/LoginDialog/CompleteProfileBody.qml | 2 +- .../qml/LoginDialog/LinkAccountBody.qml | 2 +- .../resources/qml/LoginDialog/SignUpBody.qml | 2 +- .../qml/LoginDialog/UsernameCollisionBody.qml | 2 +- interface/src/Application.cpp | 40 ++++++++++++------- interface/src/Application.h | 4 +- .../scripting/WindowScriptingInterface.cpp | 7 +++- .../src/scripting/WindowScriptingInterface.h | 7 ++-- interface/src/ui/LoginDialog.cpp | 4 -- interface/src/ui/LoginDialog.h | 2 - libraries/ui/src/FileDialogHelper.cpp | 10 ++++- 11 files changed, 50 insertions(+), 32 deletions(-) diff --git a/interface/resources/qml/LoginDialog/CompleteProfileBody.qml b/interface/resources/qml/LoginDialog/CompleteProfileBody.qml index 17d6a7d3b3..f90a7d8561 100644 --- a/interface/resources/qml/LoginDialog/CompleteProfileBody.qml +++ b/interface/resources/qml/LoginDialog/CompleteProfileBody.qml @@ -398,7 +398,7 @@ Item { lineHeight: 1 lineHeightMode: Text.ProportionalHeight - onLinkActivated: loginDialog.openUrl(link); + onLinkActivated: Window.openUrl(link); Component.onCompleted: { if (termsTextMetrics.width > root.bannerWidth) { diff --git a/interface/resources/qml/LoginDialog/LinkAccountBody.qml b/interface/resources/qml/LoginDialog/LinkAccountBody.qml index 4dd05f594d..04ffe72a57 100644 --- a/interface/resources/qml/LoginDialog/LinkAccountBody.qml +++ b/interface/resources/qml/LoginDialog/LinkAccountBody.qml @@ -363,7 +363,7 @@ Item { linkColor: hifi.colors.blueAccent onLinkActivated: { Tablet.playSound(TabletEnums.ButtonClick); - loginDialog.openUrl(link); + Window.openUrl(link); lightboxPopup.titleText = "Can't Access Account"; lightboxPopup.bodyText = lightboxPopup.cantAccessBodyText; lightboxPopup.button2text = "CLOSE"; diff --git a/interface/resources/qml/LoginDialog/SignUpBody.qml b/interface/resources/qml/LoginDialog/SignUpBody.qml index 69ac2f5a6c..f1e0cfe685 100644 --- a/interface/resources/qml/LoginDialog/SignUpBody.qml +++ b/interface/resources/qml/LoginDialog/SignUpBody.qml @@ -411,7 +411,7 @@ Item { lineHeight: 1 lineHeightMode: Text.ProportionalHeight - onLinkActivated: loginDialog.openUrl(link); + onLinkActivated: Window.openUrl(link); Component.onCompleted: { if (termsTextMetrics.width > root.bannerWidth) { diff --git a/interface/resources/qml/LoginDialog/UsernameCollisionBody.qml b/interface/resources/qml/LoginDialog/UsernameCollisionBody.qml index d450b1e7bc..d2fd1dfe35 100644 --- a/interface/resources/qml/LoginDialog/UsernameCollisionBody.qml +++ b/interface/resources/qml/LoginDialog/UsernameCollisionBody.qml @@ -234,7 +234,7 @@ Item { lineHeight: 1 lineHeightMode: Text.ProportionalHeight - onLinkActivated: loginDialog.openUrl(link); + onLinkActivated: Window.openUrl(link); Component.onCompleted: { if (termsTextMetrics.width > root.bannerWidth) { diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 7ba45da1fd..39994c53ab 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2443,6 +2443,13 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo DependencyManager::get()->preloadSounds(); DependencyManager::get()->createKeyboard(); + QDesktopServices::setUrlHandler("file", this, "showUrlHandler"); + QDesktopServices::setUrlHandler("", this, "showUrlHandler"); + auto drives = QDir::drives(); + for (auto drive : drives) { + QDesktopServices::setUrlHandler(QUrl(drive.absolutePath()).scheme(), this, "showUrlHandler"); + } + _pendingIdleEvent = false; _graphicsEngine.startup(); @@ -8280,19 +8287,6 @@ void Application::packageModel() { ModelPackager::package(); } -void Application::openUrl(const QUrl& url) const { - if (!url.isEmpty()) { - if (url.scheme() == URL_SCHEME_HIFI) { - DependencyManager::get()->handleLookupString(url.toString()); - } else if (url.scheme() == URL_SCHEME_HIFIAPP) { - DependencyManager::get()->openSystemApp(url.path()); - } else { - // address manager did not handle - ask QDesktopServices to handle - QDesktopServices::openUrl(url); - } - } -} - void Application::loadDialog() { ModalDialogListener* dlg = OffscreenUi::getOpenFileNameAsync(_glWidget, tr("Open Script"), getPreviousScriptLocation(), @@ -9161,7 +9155,7 @@ void Application::readArgumentsFromLocalSocket() const { // If we received a message, try to open it as a URL if (message.length() > 0) { - qApp->openUrl(QString::fromUtf8(message)); + DependencyManager::get()->openUrl(QString::fromUtf8(message)); } } @@ -9278,6 +9272,24 @@ QString Application::getGraphicsCardType() { return GPUIdent::getInstance()->getName(); } +void Application::showUrlHandler(const QUrl& url) { + if (QThread::currentThread() != thread()) { + QMetaObject::invokeMethod(this, "showUrlHandler", Q_ARG(const QUrl&, url)); + return; + } + + ModalDialogListener* dlg = OffscreenUi::asyncQuestion("Confirm openUrl", "Do you recognize this path or code and want to open or execute it: " + url.toDisplayString()); + QObject::connect(dlg, &ModalDialogListener::response, this, [=](QVariant answer) { + QObject::disconnect(dlg, &ModalDialogListener::response, this, nullptr); + if (QMessageBox::Yes == static_cast(answer.toInt())) { + // Unset the handler, open the URL, and the reset the handler + QDesktopServices::unsetUrlHandler(url.scheme()); + QDesktopServices::openUrl(url); + QDesktopServices::setUrlHandler(url.scheme(), this, "showUrlHandler"); + } + }); +} + #if defined(Q_OS_ANDROID) void Application::beforeEnterBackground() { auto nodeList = DependencyManager::get(); diff --git a/interface/src/Application.h b/interface/src/Application.h index e0e8821037..0ce01b47da 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -404,8 +404,6 @@ public slots: static void packageModel(); - void openUrl(const QUrl& url) const; - void resetSensors(bool andReload = false); void setActiveFaceTracker() const; @@ -472,6 +470,8 @@ public slots: QString getGraphicsCardType(); + void showUrlHandler(const QUrl& url); + private slots: void onDesktopRootItemCreated(QQuickItem* qmlContext); void onDesktopRootContextCreated(QQmlContext* qmlContext); diff --git a/interface/src/scripting/WindowScriptingInterface.cpp b/interface/src/scripting/WindowScriptingInterface.cpp index 0f3d859093..2c1311924f 100644 --- a/interface/src/scripting/WindowScriptingInterface.cpp +++ b/interface/src/scripting/WindowScriptingInterface.cpp @@ -27,6 +27,7 @@ #include "MainWindow.h" #include "Menu.h" #include "OffscreenUi.h" +#include "commerce/QmlCommerce.h" static const QString DESKTOP_LOCATION = QStandardPaths::writableLocation(QStandardPaths::DesktopLocation); static const QString LAST_BROWSE_LOCATION_SETTING = "LastBrowseLocation"; @@ -134,15 +135,17 @@ void WindowScriptingInterface::disconnectedFromDomain() { void WindowScriptingInterface::openUrl(const QUrl& url) { if (!url.isEmpty()) { - if (url.scheme() == URL_SCHEME_HIFI) { + auto scheme = url.scheme(); + if (scheme == URL_SCHEME_HIFI) { DependencyManager::get()->handleLookupString(url.toString()); + } else if (scheme == URL_SCHEME_HIFIAPP) { + DependencyManager::get()->openSystemApp(url.path()); } else { #if defined(Q_OS_ANDROID) QMap args; args["url"] = url.toString(); AndroidHelper::instance().requestActivity("WebView", true, args); #else - // address manager did not handle - ask QDesktopServices to handle QDesktopServices::openUrl(url); #endif } diff --git a/interface/src/scripting/WindowScriptingInterface.h b/interface/src/scripting/WindowScriptingInterface.h index baff6444e1..77b586ec70 100644 --- a/interface/src/scripting/WindowScriptingInterface.h +++ b/interface/src/scripting/WindowScriptingInterface.h @@ -535,9 +535,10 @@ public slots: int openMessageBox(QString title, QString text, int buttons, int defaultButton); /**jsdoc - * Open a URL in the Interface window or other application, depending on the URL's scheme. If the URL starts with - * hifi:// then that URL is navigated to in Interface, otherwise the URL is opened in the application the OS - * associates with the URL's scheme (e.g., a Web browser for http://). + * Open a URL in the Interface window or other application, depending on the URL's scheme. The following schemes are supported: + * hifi (navigate to the URL in Interface), hifiapp (open a system app in Interface). Other schemes will either be handled by the OS + * (e.g. http, https, mailto) or will create a confirmation dialog asking the user to confirm that they want to try to open + * the URL. * @function Window.openUrl * @param {string} url - The URL to open. */ diff --git a/interface/src/ui/LoginDialog.cpp b/interface/src/ui/LoginDialog.cpp index b4f504822f..50acdeaa1e 100644 --- a/interface/src/ui/LoginDialog.cpp +++ b/interface/src/ui/LoginDialog.cpp @@ -279,10 +279,6 @@ void LoginDialog::createAccountFromSteam(QString username) { } } -void LoginDialog::openUrl(const QString& url) const { - QDesktopServices::openUrl(QUrl(url)); -} - void LoginDialog::linkCompleted(QNetworkReply* reply) { emit handleLinkCompleted(); } diff --git a/interface/src/ui/LoginDialog.h b/interface/src/ui/LoginDialog.h index e2fa8adc61..7c659a9320 100644 --- a/interface/src/ui/LoginDialog.h +++ b/interface/src/ui/LoginDialog.h @@ -80,8 +80,6 @@ protected slots: Q_INVOKABLE void signup(const QString& email, const QString& username, const QString& password); - Q_INVOKABLE void openUrl(const QString& url) const; - Q_INVOKABLE bool getLoginDialogPoppedUp() const; }; diff --git a/libraries/ui/src/FileDialogHelper.cpp b/libraries/ui/src/FileDialogHelper.cpp index 6d14adf1db..e6a48da01b 100644 --- a/libraries/ui/src/FileDialogHelper.cpp +++ b/libraries/ui/src/FileDialogHelper.cpp @@ -111,7 +111,15 @@ QStringList FileDialogHelper::drives() { } void FileDialogHelper::openDirectory(const QString& path) { - QDesktopServices::openUrl(path); + QString dirPath = path; + const QString FILE_SCHEME = "file://"; + if (dirPath.startsWith(FILE_SCHEME)) { + dirPath.remove(0, FILE_SCHEME.length()); + } + QFileInfo fileInfo(dirPath); + if (fileInfo.isDir()) { + QDesktopServices::openUrl(path); + } } QList FileDialogHelper::urlToList(const QUrl& url) {