From 428a58710d3fee94e0b828fe6ca96b817294ba6d Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 29 Aug 2019 13:44:49 -0700 Subject: [PATCH] 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() {