From 428a58710d3fee94e0b828fe6ca96b817294ba6d Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 29 Aug 2019 13:44:49 -0700 Subject: [PATCH 1/4] BUGZ-1365: Ensure that by default web views can't access local content --- interface/src/Application.cpp | 15 ++++++++++++--- interface/src/Application.h | 2 +- .../src/scripting/DesktopScriptingInterface.cpp | 7 +++++-- .../src/scripting/DesktopScriptingInterface.h | 3 +++ interface/src/ui/InteractiveWindow.cpp | 13 +++++++------ interface/src/ui/InteractiveWindow.h | 2 +- .../src/AbstractScriptingServicesInterface.h | 2 +- libraries/ui/src/QmlWindowClass.cpp | 6 ++---- libraries/ui/src/ui/types/ContextAwareProfile.cpp | 10 +++++++++- 9 files changed, 41 insertions(+), 19 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 9ba886fcd2..ff3e1e9abe 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -139,6 +139,7 @@ #include #include #include +#include #include #include #include @@ -3285,6 +3286,9 @@ void Application::initializeUi() { } return result.toPoint(); }); + + // BUGZ-1365 - the root context should explicitly default to being unable to load local HTML content + ContextAwareProfile::restrictContext(offscreenUi->getSurfaceContext(), true); offscreenUi->resume(); #endif connect(_window, &MainWindow::windowGeometryChanged, [this](const QRect& r){ @@ -7457,7 +7461,7 @@ void Application::addingEntityWithCertificate(const QString& certificateID, cons ledger->updateLocation(certificateID, placeName); } -void Application::registerScriptEngineWithApplicationServices(ScriptEnginePointer scriptEngine) { +void Application::registerScriptEngineWithApplicationServices(const ScriptEnginePointer& scriptEngine) { scriptEngine->setEmitScriptUpdatesFunction([this]() { SharedNodePointer entityServerNode = DependencyManager::get()->soloNodeOfType(NodeType::EntityServer); @@ -7496,9 +7500,15 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEnginePointe qScriptRegisterMetaType(scriptEngine.data(), RayToOverlayIntersectionResultToScriptValue, RayToOverlayIntersectionResultFromScriptValue); + bool clientScript = scriptEngine->isClientScript(); + #if !defined(DISABLE_QML) scriptEngine->registerGlobalObject("OffscreenFlags", getOffscreenUI()->getFlags()); - scriptEngine->registerGlobalObject("Desktop", DependencyManager::get().data()); + if (clientScript) { + scriptEngine->registerGlobalObject("Desktop", DependencyManager::get().data()); + } else { + scriptEngine->registerGlobalObject("Desktop", new DesktopScriptingInterface(scriptEngine.get(), true)); + } #endif qScriptRegisterMetaType(scriptEngine.data(), wrapperToScriptValue, wrapperFromScriptValue); @@ -7523,7 +7533,6 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEnginePointe scriptEngine->registerGetterSetter("location", LocationScriptingInterface::locationGetter, LocationScriptingInterface::locationSetter); - bool clientScript = scriptEngine->isClientScript(); scriptEngine->registerFunction("OverlayWindow", clientScript ? QmlWindowClass::constructor : QmlWindowClass::restricted_constructor); #if !defined(Q_OS_ANDROID) && !defined(DISABLE_QML) scriptEngine->registerFunction("OverlayWebWindow", clientScript ? QmlWebWindowClass::constructor : QmlWebWindowClass::restricted_constructor); diff --git a/interface/src/Application.h b/interface/src/Application.h index 20abbf2baf..cd867598c0 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -252,7 +252,7 @@ public: NodeToOctreeSceneStats* getOcteeSceneStats() { return &_octreeServerSceneStats; } virtual controller::ScriptingInterface* getControllerScriptingInterface() { return _controllerScriptingInterface; } - virtual void registerScriptEngineWithApplicationServices(ScriptEnginePointer scriptEngine) override; + virtual void registerScriptEngineWithApplicationServices(const ScriptEnginePointer& scriptEngine) override; virtual void copyCurrentViewFrustum(ViewFrustum& viewOut) const override { copyDisplayViewFrustum(viewOut); } virtual QThread* getMainThread() override { return thread(); } diff --git a/interface/src/scripting/DesktopScriptingInterface.cpp b/interface/src/scripting/DesktopScriptingInterface.cpp index a1c8e4fc6c..874b3fa42d 100644 --- a/interface/src/scripting/DesktopScriptingInterface.cpp +++ b/interface/src/scripting/DesktopScriptingInterface.cpp @@ -52,6 +52,9 @@ static const QVariantMap DOCK_AREA { { "RIGHT", DockArea::RIGHT } }; +DesktopScriptingInterface::DesktopScriptingInterface(QObject* parent, bool restricted) + : QObject(parent), _restricted(restricted) { } + int DesktopScriptingInterface::getWidth() { QSize size = qApp->getWindow()->windowHandle()->screen()->virtualSize(); return size.width(); @@ -128,7 +131,7 @@ InteractiveWindowPointer DesktopScriptingInterface::createWindow(const QString& return nullptr; } - return new InteractiveWindow(sourceUrl, properties); + return new InteractiveWindow(sourceUrl, properties, _restricted); } InteractiveWindowPointer DesktopScriptingInterface::createWindowOnThread(const QString& sourceUrl, const QVariantMap& properties, QThread* targetThread) { @@ -139,7 +142,7 @@ InteractiveWindowPointer DesktopScriptingInterface::createWindowOnThread(const Q if (!urlValidator(sourceUrl)) { return nullptr; } - InteractiveWindowPointer window = new InteractiveWindow(sourceUrl, properties); + InteractiveWindowPointer window = new InteractiveWindow(sourceUrl, properties, _restricted); window->moveToThread(targetThread); return window; } diff --git a/interface/src/scripting/DesktopScriptingInterface.h b/interface/src/scripting/DesktopScriptingInterface.h index 525fd7c803..e562a32543 100644 --- a/interface/src/scripting/DesktopScriptingInterface.h +++ b/interface/src/scripting/DesktopScriptingInterface.h @@ -54,6 +54,8 @@ class DesktopScriptingInterface : public QObject, public Dependency { Q_PROPERTY(int CLOSE_BUTTON_HIDES READ flagCloseButtonHides CONSTANT FINAL) public: + DesktopScriptingInterface(QObject* parent= nullptr, bool restricted = false); + /**jsdoc * Sets the opacity of the HUD surface. * @function Desktop.setHUDAlpha @@ -106,6 +108,7 @@ private: static QVariantMap getDockArea(); Q_INVOKABLE static QVariantMap getPresentationMode(); + const bool _restricted; }; diff --git a/interface/src/ui/InteractiveWindow.cpp b/interface/src/ui/InteractiveWindow.cpp index af3562e747..d977c45f85 100644 --- a/interface/src/ui/InteractiveWindow.cpp +++ b/interface/src/ui/InteractiveWindow.cpp @@ -19,6 +19,8 @@ #include #include +#include +#include #include #include #include @@ -135,7 +137,7 @@ void InteractiveWindow::emitMainWindowResizeEvent() { * Set at window creation. Possible flag values are provided as {@link Desktop|Desktop.ALWAYS_ON_TOP} and {@link Desktop|Desktop.CLOSE_BUTTON_HIDES}. * Additional flag values can be found on Qt's website at https://doc.qt.io/qt-5/qt.html#WindowType-enum. */ -InteractiveWindow::InteractiveWindow(const QString& sourceUrl, const QVariantMap& properties) { +InteractiveWindow::InteractiveWindow(const QString& sourceUrl, const QVariantMap& properties, bool restricted) { InteractiveWindowPresentationMode presentationMode = InteractiveWindowPresentationMode::Native; if (properties.contains(PRESENTATION_MODE_PROPERTY)) { @@ -230,11 +232,10 @@ InteractiveWindow::InteractiveWindow(const QString& sourceUrl, const QVariantMap mainWindow->addDockWidget(dockArea, _dockWidget.get()); } else { auto contextInitLambda = [&](QQmlContext* context) { -#if !defined(Q_OS_ANDROID) - // If the restricted flag is on, override the FileTypeProfile and HFWebEngineProfile objects in the - // QML surface root context with local ones - ContextAwareProfile::restrictContext(context, false); -#endif + // If the restricted flag is on, the web content will not be able to access local files + ContextAwareProfile::restrictContext(context, restricted); + FileTypeProfile::registerWithContext(context); + HFWebEngineProfile::registerWithContext(context); }; auto objectInitLambda = [&](QQmlContext* context, QObject* object) { diff --git a/interface/src/ui/InteractiveWindow.h b/interface/src/ui/InteractiveWindow.h index cc7536af85..70077a27b0 100644 --- a/interface/src/ui/InteractiveWindow.h +++ b/interface/src/ui/InteractiveWindow.h @@ -126,7 +126,7 @@ class InteractiveWindow : public QObject { Q_PROPERTY(int presentationMode READ getPresentationMode WRITE setPresentationMode) public: - InteractiveWindow(const QString& sourceUrl, const QVariantMap& properties); + InteractiveWindow(const QString& sourceUrl, const QVariantMap& properties, bool restricted); ~InteractiveWindow(); private: diff --git a/libraries/script-engine/src/AbstractScriptingServicesInterface.h b/libraries/script-engine/src/AbstractScriptingServicesInterface.h index e7a8d11562..ac26b166b6 100644 --- a/libraries/script-engine/src/AbstractScriptingServicesInterface.h +++ b/libraries/script-engine/src/AbstractScriptingServicesInterface.h @@ -18,7 +18,7 @@ class AbstractScriptingServicesInterface { public: /// Registers application specific services with a script engine. - virtual void registerScriptEngineWithApplicationServices(ScriptEnginePointer scriptEngine) = 0; + virtual void registerScriptEngineWithApplicationServices(const ScriptEnginePointer& scriptEngine) = 0; }; diff --git a/libraries/ui/src/QmlWindowClass.cpp b/libraries/ui/src/QmlWindowClass.cpp index 4690a720f2..abce5479c4 100644 --- a/libraries/ui/src/QmlWindowClass.cpp +++ b/libraries/ui/src/QmlWindowClass.cpp @@ -137,10 +137,8 @@ void QmlWindowClass::initQml(QVariantMap properties) { // If the restricted flag is on, override the FileTypeProfile and HFWebEngineProfile objects in the // QML surface root context with local ones ContextAwareProfile::restrictContext(context, _restricted); - if (_restricted) { - FileTypeProfile::registerWithContext(context); - HFWebEngineProfile::registerWithContext(context); - } + FileTypeProfile::registerWithContext(context); + HFWebEngineProfile::registerWithContext(context); #endif }; diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.cpp b/libraries/ui/src/ui/types/ContextAwareProfile.cpp index 5efeba0602..f74e8754c9 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.cpp +++ b/libraries/ui/src/ui/types/ContextAwareProfile.cpp @@ -36,7 +36,15 @@ bool ContextAwareProfile::isRestrictedInternal() { BLOCKING_INVOKE_METHOD(this, "isRestrictedInternal", Q_RETURN_ARG(bool, restrictedResult)); return restrictedResult; } - return _context->contextProperty(RESTRICTED_FLAG_PROPERTY).toBool(); + + QVariant variant = _context->contextProperty(RESTRICTED_FLAG_PROPERTY); + if (variant.isValid()) { + return variant.toBool(); + } + + // BUGZ-1365 - we MUST defalut to restricted mode in the absence of a flag, or it's too easy for someone to make + // a new mechanism for loading web content that fails to restrict access to local files + return true; } bool ContextAwareProfile::isRestricted() { From 31340d278add67cdd95cb598b726bdb454911c76 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 29 Aug 2019 13:45:43 -0700 Subject: [PATCH 2/4] Add test for local file access --- .../qml/hifi/tablet/DynamicWebview.qml | 11 ++++ .../developer/tests/ui/testLocalFileAccess.js | 55 +++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 interface/resources/qml/hifi/tablet/DynamicWebview.qml create mode 100644 scripts/developer/tests/ui/testLocalFileAccess.js diff --git a/interface/resources/qml/hifi/tablet/DynamicWebview.qml b/interface/resources/qml/hifi/tablet/DynamicWebview.qml new file mode 100644 index 0000000000..4343e71e64 --- /dev/null +++ b/interface/resources/qml/hifi/tablet/DynamicWebview.qml @@ -0,0 +1,11 @@ +import QtQuick 2.0 +import "../../controls" as Controls + +Controls.WebView { + id: root + function fromScript(message) { + root.url = message.url; + } +} + + diff --git a/scripts/developer/tests/ui/testLocalFileAccess.js b/scripts/developer/tests/ui/testLocalFileAccess.js new file mode 100644 index 0000000000..0141d5a95f --- /dev/null +++ b/scripts/developer/tests/ui/testLocalFileAccess.js @@ -0,0 +1,55 @@ +"use strict"; + +// Created by Bradley Austin Davis on 2019/08/29 +// Copyright 2013-2019 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +/* eslint indent: ["error", 4, { "outerIIFEBody": 0 }] */ + +(function() { // BEGIN LOCAL_SCOPE + + +var QML_URL = "qrc:/qml/hifi/tablet/DynamicWebview.qml" +var LOCAL_FILE_URL = "file:///C:/test-file.html" + +var TABLET_BUTTON_NAME = "SCRIPT"; +var ICONS = { + icon: "icons/tablet-icons/menu-i.svg", + activeIcon: "icons/tablet-icons/meni-a.svg" +}; + + +var tablet = Tablet.getTablet("com.highfidelity.interface.tablet.system"); +var button = tablet.addButton({ + icon: ICONS.icon, + activeIcon: ICONS.activeIcon, + text: TABLET_BUTTON_NAME, + sortOrder: 1 +}); + +function onClicked() { + var window = Desktop.createWindow(QML_URL, { + title: "Local file access test", + additionalFlags: Desktop.ALWAYS_ON_TOP, + presentationMode: Desktop.PresentationMode.NATIVE, + size: { x: 640, y: 480 }, + visible: true, + position: { x: 100, y: 100 }, + }); + window.sendToQml({ url: LOCAL_FILE_URL }); +} + +button.clicked.connect(onClicked); + +Script.scriptEnding.connect(function () { + button.clicked.disconnect(onClicked); + tablet.removeButton(button); +}); + +}()); // END LOCAL_SCOPE + + + From 6aa967f29fed5caa73550f4425b64cd79bee0917 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 29 Aug 2019 14:11:22 -0700 Subject: [PATCH 3/4] fixing test code --- interface/src/Application.cpp | 6 +++- .../tests/ui/testLocalFileAccessServer.js | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 scripts/developer/tests/ui/testLocalFileAccessServer.js diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index ff3e1e9abe..c575845113 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -7507,7 +7507,11 @@ void Application::registerScriptEngineWithApplicationServices(const ScriptEngine if (clientScript) { scriptEngine->registerGlobalObject("Desktop", DependencyManager::get().data()); } else { - scriptEngine->registerGlobalObject("Desktop", new DesktopScriptingInterface(scriptEngine.get(), true)); + auto desktopScriptingInterface = new DesktopScriptingInterface(nullptr, true); + scriptEngine->registerGlobalObject("Desktop", desktopScriptingInterface); + if (QThread::currentThread() != thread()) { + desktopScriptingInterface->moveToThread(thread()); + } } #endif diff --git a/scripts/developer/tests/ui/testLocalFileAccessServer.js b/scripts/developer/tests/ui/testLocalFileAccessServer.js new file mode 100644 index 0000000000..74fd52a216 --- /dev/null +++ b/scripts/developer/tests/ui/testLocalFileAccessServer.js @@ -0,0 +1,30 @@ +"use strict"; + +// Created by Bradley Austin Davis on 2019/08/29 +// Copyright 2013-2019 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +(function () { + var QML_URL = "qrc:/qml/hifi/tablet/DynamicWebview.qml" + var LOCAL_FILE_URL = "file:///C:/test-file.html" + + var _this = this; + _this.preload = function (pEntityID) { + var window = Desktop.createWindow(QML_URL, { + title: "Local file access test", + additionalFlags: Desktop.ALWAYS_ON_TOP, + presentationMode: Desktop.PresentationMode.NATIVE, + size: { x: 640, y: 480 }, + visible: true, + position: { x: 100, y: 100 }, + }); + window.sendToQml({ url: LOCAL_FILE_URL }); + }; + + _this.unload = function () { + console.log("QQQ on preload"); + }; +}); From 82a49d4641642ca232e84390d1b9f51055a6124c Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 29 Aug 2019 15:46:50 -0700 Subject: [PATCH 4/4] Fix android build --- interface/src/ui/InteractiveWindow.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/interface/src/ui/InteractiveWindow.cpp b/interface/src/ui/InteractiveWindow.cpp index d977c45f85..e63c392a47 100644 --- a/interface/src/ui/InteractiveWindow.cpp +++ b/interface/src/ui/InteractiveWindow.cpp @@ -234,8 +234,10 @@ InteractiveWindow::InteractiveWindow(const QString& sourceUrl, const QVariantMap auto contextInitLambda = [&](QQmlContext* context) { // If the restricted flag is on, the web content will not be able to access local files ContextAwareProfile::restrictContext(context, restricted); +#if !defined(Q_OS_ANDROID) FileTypeProfile::registerWithContext(context); HFWebEngineProfile::registerWithContext(context); +#endif }; auto objectInitLambda = [&](QQmlContext* context, QObject* object) {