From 7b7ec5a174a0b1120278869beb9f044b5f8791ff Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 4 Apr 2016 16:41:21 -0700 Subject: [PATCH 1/2] Prohibit entity script file includes from outside the original entity script's directory (unless its in our resources). --- libraries/script-engine/src/ScriptEngine.cpp | 70 ++++++++++++++------ libraries/script-engine/src/ScriptEngine.h | 7 +- 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 31047dd423..15690baa0f 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -578,7 +578,7 @@ void ScriptEngine::addEventHandler(const EntityItemID& entityID, const QString& _registeredHandlers[entityID] = RegisteredEventHandlers(); } CallbackList& handlersForEvent = _registeredHandlers[entityID][eventName]; - CallbackData handlerData = {handler, currentEntityIdentifier}; + CallbackData handlerData = {handler, currentEntityIdentifier, currentSandboxURL}; handlersForEvent << handlerData; // Note that the same handler can be added many times. See removeEntityEventHandler(). } @@ -795,7 +795,7 @@ void ScriptEngine::timerFired() { // call the associated JS function, if it exists if (timerData.function.isValid()) { - callWithEnvironment(timerData.definingEntityIdentifier, timerData.function, timerData.function, QScriptValueList()); + callWithEnvironment(timerData.definingEntityIdentifier, timerData.definingSandboxURL, timerData.function, timerData.function, QScriptValueList()); } } @@ -810,7 +810,7 @@ QObject* ScriptEngine::setupTimerWithInterval(const QScriptValue& function, int // make sure the timer stops when the script does connect(this, &ScriptEngine::scriptEnding, newTimer, &QTimer::stop); - CallbackData timerData = {function, currentEntityIdentifier}; + CallbackData timerData = {function, currentEntityIdentifier, currentSandboxURL}; _timerFunctionMap.insert(newTimer, timerData); newTimer->start(intervalMS); @@ -885,19 +885,46 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac return; // bail early } QList urls; + bool knowsSensitivity = false; + Qt::CaseSensitivity sensitivity; + auto getSensitivity = [&]() { + if (!knowsSensitivity) { + QString path = currentSandboxURL.path(); + QFileInfo upperFI(path.toUpper()); + QFileInfo lowerFI(path.toLower()); + sensitivity = (upperFI == lowerFI) ? Qt::CaseInsensitive : Qt::CaseSensitive; + knowsSensitivity = true; + } + return sensitivity; + }; + for (QString file : includeFiles) { QUrl thisURL { resolvePath(file) }; if (!_includedURLs.contains(thisURL)) { - urls.append(thisURL); - _includedURLs << thisURL; - } - else { + if (!currentSandboxURL.isEmpty() && (thisURL.scheme() == "file") && + ( + (currentSandboxURL.scheme() != "file") || + ( + !thisURL.toString(QUrl::RemoveFilename).startsWith(defaultScriptsLocation().toString(), getSensitivity()) && + !thisURL.toString(QUrl::RemoveFilename).startsWith(currentSandboxURL.toString(QUrl::RemoveFilename), getSensitivity()) + ) + ) + ) { + qCWarning(scriptengine) << "Script.include() ignoring file path" << thisURL << "outside of original entity script" << currentSandboxURL; + } else { + // We could also check here for CORS, but we don't yet. + // It turns out that QUrl.resolve will not change hosts and copy authority, so we don't need to check that here. + urls.append(thisURL); + _includedURLs << thisURL; + } + } else { qCDebug(scriptengine) << "Script.include() ignoring previously included url:" << thisURL; } } BatchLoader* loader = new BatchLoader(urls); EntityItemID capturedEntityIdentifier = currentEntityIdentifier; + QUrl capturedSandboxURL = currentSandboxURL; auto evaluateScripts = [=](const QMap& data) { auto parentURL = _parentURL; @@ -912,13 +939,13 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac auto operation = [&]() { evaluate(contents, url.toString()); }; - doWithEnvironment(capturedEntityIdentifier, operation); + doWithEnvironment(capturedEntityIdentifier, capturedSandboxURL, operation); } } _parentURL = parentURL; if (callback.isFunction()) { - callWithEnvironment(capturedEntityIdentifier, QScriptValue(callback), QScriptValue(), QScriptValueList()); + callWithEnvironment(capturedEntityIdentifier, capturedSandboxURL, QScriptValue(callback), QScriptValue(), QScriptValueList()); } loader->deleteLater(); @@ -996,10 +1023,11 @@ void ScriptEngine::forwardHandlerCall(const EntityItemID& entityID, const QStrin CallbackList handlersForEvent = handlersOnEntity[eventName]; if (!handlersForEvent.isEmpty()) { for (int i = 0; i < handlersForEvent.count(); ++i) { - // handlersForEvent[i] can tonain many handlers that may have each been added by different interface or entity scripts, + // handlersForEvent[i] can contain many handlers that may have each been added by different interface or entity scripts, // and the entity scripts may be for entities other than the one this is a handler for. // Fortunately, the definingEntityIdentifier captured the entity script id (if any) when the handler was added. - callWithEnvironment(handlersForEvent[i].definingEntityIdentifier, handlersForEvent[i].function, QScriptValue(), eventHandlerArgs); + CallbackData& handler = handlersForEvent[i]; + callWithEnvironment(handler.definingEntityIdentifier, handler.definingSandboxURL, handler.function, QScriptValue(), eventHandlerArgs); } } } @@ -1106,13 +1134,14 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co lastModified = (quint64)QFileInfo(file).lastModified().toMSecsSinceEpoch(); } QScriptValue entityScriptConstructor, entityScriptObject; + QUrl sandboxURL = currentSandboxURL.isEmpty() ? scriptOrURL : currentSandboxURL; auto initialization = [&]{ entityScriptConstructor = evaluate(contents, fileName); entityScriptObject = entityScriptConstructor.construct(); }; - doWithEnvironment(entityID, initialization); + doWithEnvironment(entityID, sandboxURL, initialization); - EntityScriptDetails newDetails = { scriptOrURL, entityScriptObject, lastModified }; + EntityScriptDetails newDetails = { scriptOrURL, entityScriptObject, lastModified, sandboxURL }; _entityScripts[entityID] = newDetails; if (isURL) { setParentURL(""); @@ -1201,9 +1230,11 @@ void ScriptEngine::refreshFileScript(const EntityItemID& entityID) { // Even if entityID is supplied as currentEntityIdentifier, this still documents the source // of the code being executed (e.g., if we ever sandbox different entity scripts, or provide different // global values for different entity scripts). -void ScriptEngine::doWithEnvironment(const EntityItemID& entityID, std::function operation) { +void ScriptEngine::doWithEnvironment(const EntityItemID& entityID, const QUrl& sandboxURL, std::function operation) { EntityItemID oldIdentifier = currentEntityIdentifier; + QUrl oldSandboxURL = currentSandboxURL; currentEntityIdentifier = entityID; + currentSandboxURL = sandboxURL; #if DEBUG_CURRENT_ENTITY QScriptValue oldData = this->globalObject().property("debugEntityID"); @@ -1215,12 +1246,13 @@ void ScriptEngine::doWithEnvironment(const EntityItemID& entityID, std::function #endif currentEntityIdentifier = oldIdentifier; + currentSandboxURL = oldSandboxURL; } -void ScriptEngine::callWithEnvironment(const EntityItemID& entityID, QScriptValue function, QScriptValue thisObject, QScriptValueList args) { +void ScriptEngine::callWithEnvironment(const EntityItemID& entityID, const QUrl& sandboxURL, QScriptValue function, QScriptValue thisObject, QScriptValueList args) { auto operation = [&]() { function.call(thisObject, args); }; - doWithEnvironment(entityID, operation); + doWithEnvironment(entityID, sandboxURL, operation); } void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QString& methodName, const QStringList& params) { @@ -1249,7 +1281,7 @@ void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QS QScriptValueList args; args << entityID.toScriptValue(this); args << qScriptValueFromSequence(this, params); - callWithEnvironment(entityID, entityScript.property(methodName), entityScript, args); + callWithEnvironment(entityID, details.definingSandboxURL, entityScript.property(methodName), entityScript, args); } } @@ -1281,7 +1313,7 @@ void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QS QScriptValueList args; args << entityID.toScriptValue(this); args << event.toScriptValue(this); - callWithEnvironment(entityID, entityScript.property(methodName), entityScript, args); + callWithEnvironment(entityID, details.definingSandboxURL, entityScript.property(methodName), entityScript, args); } } } @@ -1315,7 +1347,7 @@ void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QS args << entityID.toScriptValue(this); args << otherID.toScriptValue(this); args << collisionToScriptValue(this, collision); - callWithEnvironment(entityID, entityScript.property(methodName), entityScript, args); + callWithEnvironment(entityID, details.definingSandboxURL, entityScript.property(methodName), entityScript, args); } } } diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 8dfc9c9abe..a6a623e751 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -47,6 +47,7 @@ class CallbackData { public: QScriptValue function; EntityItemID definingEntityIdentifier; + QUrl definingSandboxURL; }; typedef QList CallbackList; @@ -57,6 +58,7 @@ public: QString scriptText; QScriptValue scriptObject; int64_t lastModified; + QUrl definingSandboxURL; }; class ScriptEngine : public QScriptEngine, public ScriptUser, public EntitiesScriptEngineProvider { @@ -214,8 +216,9 @@ protected: Q_INVOKABLE void entityScriptContentAvailable(const EntityItemID& entityID, const QString& scriptOrURL, const QString& contents, bool isURL, bool success); EntityItemID currentEntityIdentifier {}; // Contains the defining entity script entity id during execution, if any. Empty for interface script execution. - void doWithEnvironment(const EntityItemID& entityID, std::function operation); - void callWithEnvironment(const EntityItemID& entityID, QScriptValue function, QScriptValue thisObject, QScriptValueList args); + QUrl currentSandboxURL {}; // The toplevel url string for the entity script that loaded the code being executed, else empty. + void doWithEnvironment(const EntityItemID& entityID, const QUrl& sandboxURL, std::function operation); + void callWithEnvironment(const EntityItemID& entityID, const QUrl& sandboxURL, QScriptValue function, QScriptValue thisObject, QScriptValueList args); friend class ScriptEngines; static std::atomic _stoppingAllScripts; From f35c59ce5f26b3258cb0369bc8f03b103321ddd3 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 6 Apr 2016 15:21:43 -0700 Subject: [PATCH 2/2] Guard against meaningless query and fragment parts. --- libraries/script-engine/src/ScriptEngine.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 15690baa0f..663ad51dbf 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -898,6 +898,9 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac return sensitivity; }; + // Guard against meaningless query and fragment parts. + // Do NOT use PreferLocalFile as its behavior is unpredictable (e.g., on defaultScriptsLocation()) + const auto strippingFlags = QUrl::RemoveFilename | QUrl::RemoveQuery | QUrl::RemoveFragment; for (QString file : includeFiles) { QUrl thisURL { resolvePath(file) }; if (!_includedURLs.contains(thisURL)) { @@ -905,13 +908,13 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac ( (currentSandboxURL.scheme() != "file") || ( - !thisURL.toString(QUrl::RemoveFilename).startsWith(defaultScriptsLocation().toString(), getSensitivity()) && - !thisURL.toString(QUrl::RemoveFilename).startsWith(currentSandboxURL.toString(QUrl::RemoveFilename), getSensitivity()) + !thisURL.toString(strippingFlags).startsWith(defaultScriptsLocation().toString(), getSensitivity()) && + !thisURL.toString(strippingFlags).startsWith(currentSandboxURL.toString(strippingFlags), getSensitivity()) ) ) ) { qCWarning(scriptengine) << "Script.include() ignoring file path" << thisURL << "outside of original entity script" << currentSandboxURL; - } else { + } else { // We could also check here for CORS, but we don't yet. // It turns out that QUrl.resolve will not change hosts and copy authority, so we don't need to check that here. urls.append(thisURL);