From 31b3f0e8f1faa8f9e4a2d89c2375607deaed2fb3 Mon Sep 17 00:00:00 2001
From: SamGondelman <samuel_gondelman@alumni.brown.edu>
Date: Tue, 23 Apr 2019 17:34:46 -0700
Subject: [PATCH 1/2] 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<TabletScriptingInterface>()->preloadSounds();
DependencyManager::get<Keyboard>()->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<AddressManager>()->handleLookupString(url.toString());
- } else if (url.scheme() == URL_SCHEME_HIFIAPP) {
- DependencyManager::get<QmlCommerce>()->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<WindowScriptingInterface>()->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<QMessageBox::StandardButton>(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<NodeList>();
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<AddressManager>()->handleLookupString(url.toString());
+ } else if (scheme == URL_SCHEME_HIFIAPP) {
+ DependencyManager::get<QmlCommerce>()->openSystemApp(url.path());
} else {
#if defined(Q_OS_ANDROID)
QMap<QString, QString> 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
- * <code>hifi://</code> 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 <code>http://</code>).
+ * Open a URL in the Interface window or other application, depending on the URL's scheme. The following schemes are supported:
+ * <code>hifi</code> (navigate to the URL in Interface), <code>hifiapp<code> (open a system app in Interface). Other schemes will either be handled by the OS
+ * (e.g. <code>http</code>, <code>https</code>, <code>mailto</code>) 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<QUrl> FileDialogHelper::urlToList(const QUrl& url) {
From 0e4ea4aff2109b24be8dee9f56dbcbd8f57bd272 Mon Sep 17 00:00:00 2001
From: SamGondelman <samuel_gondelman@alumni.brown.edu>
Date: Thu, 25 Apr 2019 12:24:02 -0700
Subject: [PATCH 2/2] fix openDirectory
---
interface/src/Application.cpp | 21 +++++++++++++++++++++
interface/src/Application.h | 2 ++
libraries/ui/src/FileDialogHelper.cpp | 11 +++--------
libraries/ui/src/FileDialogHelper.h | 3 +++
4 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index 39994c53ab..6a6bc08692 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -2443,6 +2443,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo
DependencyManager::get<TabletScriptingInterface>()->preloadSounds();
DependencyManager::get<Keyboard>()->createKeyboard();
+ FileDialogHelper::setOpenDirectoryOperator([this](const QString& path) { openDirectory(path); });
QDesktopServices::setUrlHandler("file", this, "showUrlHandler");
QDesktopServices::setUrlHandler("", this, "showUrlHandler");
auto drives = QDir::drives();
@@ -9272,6 +9273,26 @@ QString Application::getGraphicsCardType() {
return GPUIdent::getInstance()->getName();
}
+void Application::openDirectory(const QString& path) {
+ if (QThread::currentThread() != thread()) {
+ QMetaObject::invokeMethod(this, "openDirectory", Q_ARG(const QString&, path));
+ return;
+ }
+
+ 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()) {
+ auto scheme = QUrl(path).scheme();
+ QDesktopServices::unsetUrlHandler(scheme);
+ QDesktopServices::openUrl(path);
+ QDesktopServices::setUrlHandler(scheme, this, "showUrlHandler");
+ }
+}
+
void Application::showUrlHandler(const QUrl& url) {
if (QThread::currentThread() != thread()) {
QMetaObject::invokeMethod(this, "showUrlHandler", Q_ARG(const QUrl&, url));
diff --git a/interface/src/Application.h b/interface/src/Application.h
index 0ce01b47da..d2c59343ed 100644
--- a/interface/src/Application.h
+++ b/interface/src/Application.h
@@ -345,6 +345,8 @@ public:
void toggleAwayMode();
#endif
+ void openDirectory(const QString& path);
+
signals:
void svoImportRequested(const QString& url);
diff --git a/libraries/ui/src/FileDialogHelper.cpp b/libraries/ui/src/FileDialogHelper.cpp
index e6a48da01b..6d16a1ec21 100644
--- a/libraries/ui/src/FileDialogHelper.cpp
+++ b/libraries/ui/src/FileDialogHelper.cpp
@@ -17,6 +17,7 @@
#include <QtCore/QRegularExpression>
#include <QDesktopServices>
+std::function<void(const QString&)> FileDialogHelper::_openDirectoryOperator = nullptr;
QUrl FileDialogHelper::home() {
return pathToUrl(QStandardPaths::standardLocations(QStandardPaths::HomeLocation)[0]);
@@ -111,14 +112,8 @@ QStringList FileDialogHelper::drives() {
}
void FileDialogHelper::openDirectory(const QString& 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);
+ if (_openDirectoryOperator) {
+ _openDirectoryOperator(path);
}
}
diff --git a/libraries/ui/src/FileDialogHelper.h b/libraries/ui/src/FileDialogHelper.h
index 12fd60daac..4685c3af6f 100644
--- a/libraries/ui/src/FileDialogHelper.h
+++ b/libraries/ui/src/FileDialogHelper.h
@@ -16,6 +16,7 @@
#include <QtCore/QStringList>
#include <QtCore/QUrl>
+#include <functional>
class FileDialogHelper : public QObject {
Q_OBJECT
@@ -62,6 +63,7 @@ public:
Q_INVOKABLE QUrl saveHelper(const QString& saveText, const QUrl& currentFolder, const QStringList& selectionFilters);
Q_INVOKABLE QList<QUrl> urlToList(const QUrl& url);
+ static void setOpenDirectoryOperator(std::function<void(const QString&)> openDirectoryOperator) { _openDirectoryOperator = openDirectoryOperator; }
Q_INVOKABLE void openDirectory(const QString& path);
Q_INVOKABLE void monitorDirectory(const QString& path);
@@ -72,6 +74,7 @@ signals:
private:
QFileSystemWatcher _fsWatcher;
QString _fsWatcherPath;
+ static std::function<void(const QString&)> _openDirectoryOperator;
};