From 225578febe95518d1de2f0c7a2f651c1e439c4d4 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sat, 23 Mar 2024 01:19:05 +0100 Subject: [PATCH 1/9] Added simple protection for avatar URL --- .../hifi/dialogs/security/ScriptSecurity.qml | 178 ++++++++++++++++++ interface/src/Application.cpp | 2 +- interface/src/Menu.cpp | 13 ++ interface/src/Menu.h | 1 + interface/src/avatar/MyAvatar.cpp | 4 + interface/src/avatar/MyAvatar.h | 1 + libraries/avatars/src/AvatarData.cpp | 12 ++ libraries/avatars/src/AvatarData.h | 2 +- libraries/avatars/src/ScriptAvatarData.cpp | 10 +- .../networking/src/NetworkingConstants.h | 2 + libraries/script-engine/src/ScriptManager.cpp | 4 + libraries/script-engine/src/ScriptManager.h | 7 + .../script-engine/src/ScriptPermissions.cpp | 102 ++++++++++ .../script-engine/src/ScriptPermissions.h | 31 +++ libraries/shared/src/SpatiallyNestable.h | 1 + 15 files changed, 367 insertions(+), 3 deletions(-) create mode 100644 interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml create mode 100644 libraries/script-engine/src/ScriptPermissions.cpp create mode 100644 libraries/script-engine/src/ScriptPermissions.h diff --git a/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml b/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml new file mode 100644 index 0000000000..b36872d387 --- /dev/null +++ b/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml @@ -0,0 +1,178 @@ +// +// ScriptPermissions.cpp +// libraries/script-engine/src/ScriptPermissions.cpp +// +// Created by dr Karol Suprynowicz on 2024/03/24. +// Copyright 2024 Overte e.V. +// +// Based on EntityScriptQMLWhitelist.qml +// Created by Kalila L. on 2019.12.05 | realities.dev | somnilibertas@gmail.com +// Copyright 2019 Kalila L. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// +// Security settings for the script engines + +import Hifi 1.0 as Hifi +import QtQuick 2.8 +import QtQuick.Controls 2.3 +import QtQuick.Layouts 1.12 +import stylesUit 1.0 as HifiStylesUit +import controlsUit 1.0 as HiFiControls +import PerformanceEnums 1.0 +import "../../../windows" + + +Rectangle { + id: parentBody; + + function getWhitelistAsText() { + var whitelist = Settings.getValue("private/scriptPermissionGetAvatarURLSafeURLs"); + var arrayWhitelist = whitelist.split(",").join("\n"); + return arrayWhitelist; + } + + function setWhitelistAsText(whitelistText) { + Settings.setValue("private/scriptPermissionGetAvatarURLSafeURLs", whitelistText.text); + + var originalSetString = whitelistText.text; + var originalSet = originalSetString.split(' ').join(''); + + var check = Settings.getValue("private/scriptPermissionGetAvatarURLSafeURLs"); + var arrayCheck = check.split(",").join("\n"); + + setWhitelistSuccess(arrayCheck === originalSet); + } + + function setWhitelistSuccess(success) { + if (success) { + notificationText.text = "Successfully saved settings."; + } else { + notificationText.text = "Error! Settings not saved."; + } + } + + function toggleWhitelist(enabled) { + Settings.setValue("private/scriptPermissionGetAvatarURLEnable", enabled); + console.info("Toggling Protect Avatar URLs to:", enabled); + } + + function initCheckbox() { + var check = Settings.getValue("private/scriptPermissionGetAvatarURLEnable", true); + + if (check) { + whitelistEnabled.toggle(); + } + } + + + anchors.fill: parent + width: parent.width; + height: 120; + color: "#80010203"; + + HifiStylesUit.RalewayRegular { + id: titleText; + text: "Protect Avatar URLs" + // Text size + size: 24; + // Style + color: "white"; + elide: Text.ElideRight; + // Anchors + anchors.top: parent.top; + anchors.left: parent.left; + anchors.leftMargin: 20; + anchors.right: parent.right; + anchors.rightMargin: 20; + height: 60; + + CheckBox { + Component.onCompleted: { + initCheckbox(); + } + + id: whitelistEnabled; + + anchors.right: parent.right; + anchors.top: parent.top; + anchors.topMargin: 10; + onToggled: { + toggleWhitelist(whitelistEnabled.checked) + } + + Label { + text: "Enabled" + color: "white" + font.pixelSize: 18; + anchors.right: parent.left; + anchors.top: parent.top; + anchors.topMargin: 10; + } + } + } + + Rectangle { + id: textAreaRectangle; + color: "black"; + width: parent.width; + height: 250; + anchors.top: titleText.bottom; + + ScrollView { + id: textAreaScrollView + anchors.fill: parent; + width: parent.width + height: parent.height + contentWidth: parent.width + contentHeight: parent.height + clip: false; + + TextArea { + id: whitelistTextArea + text: getWhitelistAsText(); + onTextChanged: notificationText.text = ""; + width: parent.width; + height: parent.height; + font.family: "Ubuntu"; + font.pointSize: 12; + color: "white"; + } + } + + Button { + id: saveChanges + anchors.topMargin: 5; + anchors.leftMargin: 20; + anchors.rightMargin: 20; + x: textAreaRectangle.x + textAreaRectangle.width - width - 15; + y: textAreaRectangle.y + textAreaRectangle.height - height; + contentItem: Text { + text: saveChanges.text + font.family: "Ubuntu"; + font.pointSize: 12; + opacity: enabled ? 1.0 : 0.3 + color: "black" + horizontalAlignment: Text.AlignHCenter + verticalAlignment: Text.AlignVCenter + elide: Text.ElideRight + } + text: "Save Changes" + onClicked: setWhitelistAsText(whitelistTextArea) + + HifiStylesUit.RalewayRegular { + id: notificationText; + text: "" + // Text size + size: 16; + // Style + color: "white"; + elide: Text.ElideLeft; + // Anchors + anchors.right: parent.left; + anchors.rightMargin: 10; + } + } + } +} diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 137457c722..431bd2cb96 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -3305,7 +3305,7 @@ void Application::initializeUi() { // END PULL SAFEURLS FROM INTERFACE.JSON Settings - if (AUTHORIZED_EXTERNAL_QML_SOURCE.isParentOf(url)) { + if (QUrl(NetworkingConstants::OVERTE_COMMUNITY_APPLICATIONS).isParentOf(url)) { return true; } else { for (const auto& str : safeURLS) { diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index aae6839282..7017f2a083 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -323,6 +323,19 @@ Menu::Menu() { } }); + // Settings > Script Security + action = addActionToQMenuAndActionHash(settingsMenu, MenuOption::ScriptSecurity); + connect(action, &QAction::triggered, [] { + auto tablet = DependencyManager::get()->getTablet("com.highfidelity.interface.tablet.system"); + auto hmd = DependencyManager::get(); + + tablet->pushOntoStack("hifi/dialogs/security/ScriptSecurity.qml"); + + if (!hmd->getShouldShowTablet()) { + hmd->toggleShouldShowTablet(); + } + }); + // Settings > Developer Menu addCheckableActionToQMenuAndActionHash(settingsMenu, "Developer Menu", 0, false, this, SLOT(toggleDeveloperMenus())); diff --git a/interface/src/Menu.h b/interface/src/Menu.h index 931c00cfd9..e0cdfdf4fd 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -190,6 +190,7 @@ namespace MenuOption { const QString RunTimingTests = "Run Timing Tests"; const QString ScriptedMotorControl = "Enable Scripted Motor Control"; const QString EntityScriptQMLWhitelist = "Entity Script / QML Whitelist"; + const QString ScriptSecurity = "Script Security"; const QString ShowTrackedObjects = "Show Tracked Objects"; const QString SelfieCamera = "Selfie"; const QString SendWrongDSConnectVersion = "Send wrong DS connect version"; diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 62100f9cae..dc8963689a 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -73,6 +73,7 @@ #include "MovingEntitiesOperator.h" #include "SceneScriptingInterface.h" #include "WarningsSuppression.h" +#include "ScriptPermissions.h" using namespace std; @@ -2236,6 +2237,9 @@ AttachmentData MyAvatar::loadAttachmentData(const QUrl& modelURL, const QString& return attachment; } +bool MyAvatar::isMyAvatarURLProtected() const { + return !ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission::SCRIPT_PERMISSION_GET_AVATAR_URL); +} int MyAvatar::parseDataFromBuffer(const QByteArray& buffer) { qCDebug(interfaceapp) << "Error: ignoring update packet for MyAvatar" diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 5e0627360c..4198deba84 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -2683,6 +2683,7 @@ private: void setEnableDrawAverageFacing(bool drawAverage) { _drawAverageFacingEnabled = drawAverage; } bool getEnableDrawAverageFacing() const { return _drawAverageFacingEnabled; } virtual bool isMyAvatar() const override { return true; } + virtual bool isMyAvatarURLProtected() const override; virtual int parseDataFromBuffer(const QByteArray& buffer) override; virtual glm::vec3 getSkeletonPosition() const override; int _skeletonModelChangeCount { 0 }; diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index c71da50b1a..da48bced06 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2106,6 +2106,18 @@ const QUrl& AvatarData::getSkeletonModelURL() const { } } +QString AvatarData::getSkeletonModelURLFromScript() const { + if (isMyAvatar()) { + if (!isMyAvatarURLProtected()) { + return _skeletonModelURL.toString(); + } else { + return QString(); + } + } else { + return QString(); + } +}; + QByteArray AvatarData::packSkeletonData() const { // Send an avatar trait packet with the skeleton data before the mesh is loaded int avatarDataSize = 0; diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 0b2a925de0..69dd747543 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -1355,7 +1355,7 @@ public: */ Q_INVOKABLE virtual void detachAll(const QString& modelURL, const QString& jointName = QString()); - QString getSkeletonModelURLFromScript() const { return _skeletonModelURL.toString(); } + QString getSkeletonModelURLFromScript() const; void setSkeletonModelURLFromScript(const QString& skeletonModelString) { setSkeletonModelURL(QUrl(skeletonModelString)); } void setOwningAvatarMixer(const QWeakPointer& owningAvatarMixer) { _owningAvatarMixer = owningAvatarMixer; } diff --git a/libraries/avatars/src/ScriptAvatarData.cpp b/libraries/avatars/src/ScriptAvatarData.cpp index a07c402555..5a2cd2d225 100644 --- a/libraries/avatars/src/ScriptAvatarData.cpp +++ b/libraries/avatars/src/ScriptAvatarData.cpp @@ -204,7 +204,15 @@ bool ScriptAvatarData::getLookAtSnappingEnabled() const { // QString ScriptAvatarData::getSkeletonModelURLFromScript() const { if (AvatarSharedPointer sharedAvatarData = _avatarData.lock()) { - return sharedAvatarData->getSkeletonModelURLFromScript(); + if (sharedAvatarData->isMyAvatar()) { + if (sharedAvatarData->isMyAvatarURLProtected()) { + return QString(); + } else { + return sharedAvatarData->getSkeletonModelURLFromScript(); + } + } else { + return QString(); + } } else { return QString(); } diff --git a/libraries/networking/src/NetworkingConstants.h b/libraries/networking/src/NetworkingConstants.h index 4287da92d4..b10e09497a 100644 --- a/libraries/networking/src/NetworkingConstants.h +++ b/libraries/networking/src/NetworkingConstants.h @@ -58,6 +58,8 @@ namespace NetworkingConstants { const QString HF_PUBLIC_CDN_URL = ""; const QString HF_MARKETPLACE_CDN_HOSTNAME = ""; const QString OVERTE_CONTENT_CDN_URL = "https://content.overte.org/"; + const QString OVERTE_COMMUNITY_APPLICATIONS = { "https://more.overte.org/applications" }; + const QString OVERTE_TUTORIAL_SCRIPTS = { "https://more.overte.org/tutorial" }; #if USE_STABLE_GLOBAL_SERVICES const QString ICE_SERVER_DEFAULT_HOSTNAME = "ice.overte.org"; diff --git a/libraries/script-engine/src/ScriptManager.cpp b/libraries/script-engine/src/ScriptManager.cpp index 37ba7d0442..2caab052ec 100644 --- a/libraries/script-engine/src/ScriptManager.cpp +++ b/libraries/script-engine/src/ScriptManager.cpp @@ -542,6 +542,10 @@ QString ScriptManager::getFilename() const { return lastPart; } +QString ScriptManager::getAbsoluteFilename() const { + return _fileNameString; +} + bool ScriptManager::hasValidScriptSuffix(const QString& scriptFileName) { QFileInfo fileInfo(scriptFileName); QString scriptSuffixToLower = fileInfo.completeSuffix().toLower(); diff --git a/libraries/script-engine/src/ScriptManager.h b/libraries/script-engine/src/ScriptManager.h index 623b51a43f..8197f26285 100644 --- a/libraries/script-engine/src/ScriptManager.h +++ b/libraries/script-engine/src/ScriptManager.h @@ -430,6 +430,13 @@ public: */ QString getFilename() const; + /** + * @brief Get the filename of the running script, with absolute path. + * + * @return QString Filename + */ + QString getAbsoluteFilename() const; + /** * @brief Underlying scripting engine * diff --git a/libraries/script-engine/src/ScriptPermissions.cpp b/libraries/script-engine/src/ScriptPermissions.cpp new file mode 100644 index 0000000000..80bdb5ef4d --- /dev/null +++ b/libraries/script-engine/src/ScriptPermissions.cpp @@ -0,0 +1,102 @@ +// +// ScriptPermissions.cpp +// libraries/script-engine/src/ScriptPermissions.cpp +// +// Created by dr Karol Suprynowicz on 2024/03/24. +// Copyright 2024 Overte e.V. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "ScriptPermissions.h" + +#include +#include + +#include "ScriptEngine.h" +#include "ScriptManager.h" +#include "Scriptable.h" + +static const bool PERMISSIONS_DEBUG_ENABLED = false; + +extern const std::array(ScriptPermissions::Permission::SCRIPT_PERMISSIONS_SIZE)> scriptPermissionNames { + "Permission to get user's avatar URL" //SCRIPT_PERMISSION_GET_AVATAR_URL +}; + +extern const std::array(ScriptPermissions::Permission::SCRIPT_PERMISSIONS_SIZE)> scriptPermissionSettingKeyNames { + "private/scriptPermissionGetAvatarURLSafeURLs" //SCRIPT_PERMISSION_GET_AVATAR_URL +}; + +extern const std::array(ScriptPermissions::Permission::SCRIPT_PERMISSIONS_SIZE)> scriptPermissionSettingEnableKeyNames { + "private/scriptPermissionGetAvatarURLEnable" //SCRIPT_PERMISSION_GET_AVATAR_URL +}; + +extern const std::array(ScriptPermissions::Permission::SCRIPT_PERMISSIONS_SIZE)> scriptPermissionSettingEnableDefaultValues { + true //SCRIPT_PERMISSION_GET_AVATAR_URL +}; + +bool ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission permission) { + if (permission >= ScriptPermissions::Permission::SCRIPT_PERMISSIONS_SIZE) { + return false; + } + int permissionIndex = static_cast(permission); + // Check if the permission checking is active + Setting::Handle isCheckingEnabled(scriptPermissionSettingEnableKeyNames[permissionIndex], scriptPermissionSettingEnableDefaultValues[permissionIndex]); + // Get the script manager: + auto engine = Scriptable::engine(); + if (!engine) { + qDebug() << "ScriptPermissions::isCurrentScriptAllowed called outside script engine for permission: " << scriptPermissionNames[permissionIndex]; + return false; + } + auto manager = engine->manager(); + if (!manager) { + qDebug() << "ScriptPermissions::isCurrentScriptAllowed called from script engine with no script manager for permission: " << scriptPermissionNames[permissionIndex]; + return false; + } + std::vector urlsToCheck; + QString scriptURL = manager->getAbsoluteFilename(); + if (scriptURL.startsWith("about:Entities")) { + // This is entity script manager, we need to find the file name of the current script instead + scriptURL = Scriptable::context()->currentFileName(); + urlsToCheck.push_back(scriptURL); + if (PERMISSIONS_DEBUG_ENABLED) { + qDebug() << "ScriptPermissions::isCurrentScriptAllowed: filename: " << scriptURL; + } + auto parentContext = Scriptable::context()->parentContext(); + while (parentContext) { + urlsToCheck.push_back(parentContext->currentFileName()); + if (PERMISSIONS_DEBUG_ENABLED) { + qDebug() << "ScriptPermissions::isCurrentScriptAllowed: parent filename: " << parentContext->currentFileName(); + } + parentContext = parentContext->parentContext(); + } + } else { + urlsToCheck.push_back(scriptURL); + } + // Check if the script is allowed: + QList safeURLPrefixes = { "file:///", "qrc:/", NetworkingConstants::OVERTE_COMMUNITY_APPLICATIONS, + NetworkingConstants::OVERTE_TUTORIAL_SCRIPTS/*, "about:console"*/}; + Setting::Handle allowedURLsSetting(scriptPermissionSettingKeyNames[permissionIndex]); + QList allowedURLs = allowedURLsSetting.get().split("\n"); + + for (auto entry : allowedURLs) { + safeURLPrefixes.push_back(entry); + } + + for (const auto& str : safeURLPrefixes) { + if (!str.isEmpty() && scriptURL.startsWith(str)) { + if (PERMISSIONS_DEBUG_ENABLED) { + qDebug() << "ScriptPermissions::isCurrentScriptAllowed: " << scriptPermissionNames[permissionIndex] + << " for script " << scriptURL << " accepted with rule: " << str; + } + return true; + } + } + + if (PERMISSIONS_DEBUG_ENABLED) { + qDebug() << "ScriptPermissions::isCurrentScriptAllowed: " << scriptPermissionNames[permissionIndex] << " for script " + << scriptURL << " rejected."; + } + return false; +} \ No newline at end of file diff --git a/libraries/script-engine/src/ScriptPermissions.h b/libraries/script-engine/src/ScriptPermissions.h new file mode 100644 index 0000000000..f4b06253c5 --- /dev/null +++ b/libraries/script-engine/src/ScriptPermissions.h @@ -0,0 +1,31 @@ +// +// ScriptPermissions.h +// libraries/script-engine/src/ScriptPermissions.h +// +// Created by dr Karol Suprynowicz on 2024/03/24. +// Copyright 2024 Overte e.V. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#pragma once + +#include + +#include "SettingHandle.h" +#include "DependencyManager.h" + +class ScriptPermissions { +public: + enum class Permission { + SCRIPT_PERMISSION_GET_AVATAR_URL, + SCRIPT_PERMISSIONS_SIZE + }; + + static bool isCurrentScriptAllowed(Permission permission); + //TODO: add a function to request permission through a popup +}; + +// TODO: add ScriptPermissionsScriptingInterface, where script can check if they have permissions +// and request permissions through a tablet popup. diff --git a/libraries/shared/src/SpatiallyNestable.h b/libraries/shared/src/SpatiallyNestable.h index 29f23afdfb..5de86d7f95 100644 --- a/libraries/shared/src/SpatiallyNestable.h +++ b/libraries/shared/src/SpatiallyNestable.h @@ -47,6 +47,7 @@ public: virtual void setParentID(const QUuid& parentID); virtual bool isMyAvatar() const { return false; } + virtual bool isMyAvatarURLProtected() const { return false; } // This needs to be here because both MyAvatar and AvatarData inherit from MyAvatar virtual quint16 getParentJointIndex() const { return _parentJointIndex; } virtual void setParentJointIndex(quint16 parentJointIndex); From 16530b2334b87e6f01ebdcc17624cf2a9b75e639 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sat, 23 Mar 2024 20:41:22 +0100 Subject: [PATCH 2/9] Moved avatar URL to fully private settings --- .../io/highfidelity/hifiinterface/PermissionChecker.java | 2 +- interface/src/CrashRecoveryHandler.cpp | 5 ++++- interface/src/avatar/MyAvatar.cpp | 2 +- interface/src/scripting/SettingsScriptingInterface.cpp | 8 +++++++- libraries/shared/src/SettingHandle.cpp | 2 ++ libraries/shared/src/SettingHandle.h | 9 +++++++++ tools/nitpick/AppDataHighFidelity/Interface.json | 2 +- 7 files changed, 25 insertions(+), 5 deletions(-) diff --git a/android/apps/interface/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java b/android/apps/interface/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java index ef9876c71a..0703fabf02 100644 --- a/android/apps/interface/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java +++ b/android/apps/interface/src/main/java/io/highfidelity/hifiinterface/PermissionChecker.java @@ -109,7 +109,7 @@ public class PermissionChecker extends Activity { JSONObject obj = new JSONObject(); try { obj.put("firstRun",false); - obj.put("Avatar/fullAvatarURL", avatarPaths[which]); + obj.put(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/Avatar/fullAvatarURL", avatarPaths[which]); File directory = new File(pathForJson); if(!directory.exists()) directory.mkdirs(); diff --git a/interface/src/CrashRecoveryHandler.cpp b/interface/src/CrashRecoveryHandler.cpp index 1f6cbef9ba..97e03f003d 100644 --- a/interface/src/CrashRecoveryHandler.cpp +++ b/interface/src/CrashRecoveryHandler.cpp @@ -258,10 +258,11 @@ void CrashRecoveryHandler::handleCrash(CrashRecoveryHandler::Action action) { // Display name and avatar settings.beginGroup(AVATAR_GROUP); displayName = settings.value(DISPLAY_NAME_KEY).toString(); - fullAvatarURL = settings.value(FULL_AVATAR_URL_KEY).toUrl(); fullAvatarModelName = settings.value(FULL_AVATAR_MODEL_NAME_KEY).toString(); settings.endGroup(); + fullAvatarURL = settings.value(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/" + AVATAR_GROUP + "/" + FULL_AVATAR_URL_KEY).toUrl(); + // Tutorial complete tutorialComplete = settings.value(TUTORIAL_COMPLETE_FLAG_KEY).toBool(); } @@ -284,6 +285,8 @@ void CrashRecoveryHandler::handleCrash(CrashRecoveryHandler::Action action) { settings.setValue(FULL_AVATAR_MODEL_NAME_KEY, fullAvatarModelName); settings.endGroup(); + settings.setValue(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/" + AVATAR_GROUP + "/" + FULL_AVATAR_URL_KEY, fullAvatarURL); + // Tutorial complete settings.setValue(TUTORIAL_COMPLETE_FLAG_KEY, tutorialComplete); } diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index dc8963689a..21ac211e78 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -227,7 +227,7 @@ MyAvatar::MyAvatar(QThread* thread) : _yawSpeedSetting(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "yawSpeed", _yawSpeed), _hmdYawSpeedSetting(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "hmdYawSpeed", _hmdYawSpeed), _pitchSpeedSetting(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "pitchSpeed", _pitchSpeed), - _fullAvatarURLSetting(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "fullAvatarURL", + _fullAvatarURLSetting(QStringList() << SETTINGS_FULL_PRIVATE_GROUP_NAME << AVATAR_SETTINGS_GROUP_NAME << "fullAvatarURL", AvatarData::defaultFullAvatarModelUrl()), _fullAvatarModelNameSetting(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "fullAvatarModelName", _fullAvatarModelName), _animGraphURLSetting(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "animGraphURL", QUrl("")), diff --git a/interface/src/scripting/SettingsScriptingInterface.cpp b/interface/src/scripting/SettingsScriptingInterface.cpp index b7ef172f19..cf3737cd9c 100644 --- a/interface/src/scripting/SettingsScriptingInterface.cpp +++ b/interface/src/scripting/SettingsScriptingInterface.cpp @@ -25,6 +25,9 @@ QVariant SettingsScriptingInterface::getValue(const QString& setting) { if (!value.isValid()) { value = ""; } + if (_restrictPrivateValues || setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { + value = ""; + } return value; } @@ -33,6 +36,9 @@ QVariant SettingsScriptingInterface::getValue(const QString& setting, const QVar if (!value.isValid()) { value = ""; } + if (_restrictPrivateValues || setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { + value = ""; + } return value; } @@ -40,7 +46,7 @@ void SettingsScriptingInterface::setValue(const QString& setting, const QVariant if (getValue(setting) == value) { return; } - if (setting.startsWith("private/")) { + if (setting.startsWith("private/") || setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { if (_restrictPrivateValues) { qWarning() << "SettingsScriptingInterface::setValue -- restricted write: " << setting << value; return; diff --git a/libraries/shared/src/SettingHandle.cpp b/libraries/shared/src/SettingHandle.cpp index 88785e5700..2353f30933 100644 --- a/libraries/shared/src/SettingHandle.cpp +++ b/libraries/shared/src/SettingHandle.cpp @@ -18,6 +18,8 @@ Q_LOGGING_CATEGORY(settings_handle, "settings.handle") +const QString SETTINGS_FULL_PRIVATE_GROUP_NAME = "fullPrivate"; + const QString Settings::firstRun { "firstRun" }; diff --git a/libraries/shared/src/SettingHandle.h b/libraries/shared/src/SettingHandle.h index 2390063555..f8ba5f66ed 100644 --- a/libraries/shared/src/SettingHandle.h +++ b/libraries/shared/src/SettingHandle.h @@ -31,6 +31,15 @@ Q_DECLARE_LOGGING_CATEGORY(settings_handle) +/** + * @brief Name of the fully private settings group + * + * Settings in this group will be protected from reading and writing from script engines. + * + */ + +extern const QString SETTINGS_FULL_PRIVATE_GROUP_NAME; + /** * @brief QSettings analog * diff --git a/tools/nitpick/AppDataHighFidelity/Interface.json b/tools/nitpick/AppDataHighFidelity/Interface.json index a9d27d8309..bf3519fcb6 100644 --- a/tools/nitpick/AppDataHighFidelity/Interface.json +++ b/tools/nitpick/AppDataHighFidelity/Interface.json @@ -43,7 +43,7 @@ "Avatar/dominantHand": "right", "Avatar/flyingHMD": false, "Avatar/fullAvatarModelName": "Default", - "Avatar/fullAvatarURL": "", + "fullPrivate/Avatar/fullAvatarURL": "", "Avatar/headPitch": 0, "Avatar/pitchSpeed": 75, "Avatar/scale": 1, From 076bd2dbeeb26b7cb3fc5a67d986d56d724b6b66 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sun, 24 Mar 2024 22:36:26 +0100 Subject: [PATCH 3/9] Added permissions to avatar bookmarks --- interface/src/AvatarBookmarks.cpp | 42 +++++++++++++++++++ interface/src/AvatarBookmarks.h | 7 +++- .../script-engine/src/ScriptPermissions.cpp | 10 +++-- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/interface/src/AvatarBookmarks.cpp b/interface/src/AvatarBookmarks.cpp index 256ce2f6fc..461c55e64e 100644 --- a/interface/src/AvatarBookmarks.cpp +++ b/interface/src/AvatarBookmarks.cpp @@ -41,6 +41,15 @@ #include #include #include "WarningsSuppression.h" +#include "ScriptPermissions.h" + +QVariantMap AvatarBookmarks::getBookmarks() { + if (ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission::SCRIPT_PERMISSION_GET_AVATAR_URL)) { + return _bookmarks; + } else { + return {}; + } +} void addAvatarEntities(const QVariantList& avatarEntities) { auto nodeList = DependencyManager::get(); @@ -123,6 +132,12 @@ AvatarBookmarks::AvatarBookmarks() { } void AvatarBookmarks::addBookmark(const QString& bookmarkName) { + if (ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission::SCRIPT_PERMISSION_GET_AVATAR_URL)) { + addBookmarkInternal(bookmarkName); + } +} + +void AvatarBookmarks::addBookmarkInternal(const QString& bookmarkName) { if (QThread::currentThread() != thread()) { BLOCKING_INVOKE_METHOD(this, "addBookmark", Q_ARG(QString, bookmarkName)); return; @@ -134,6 +149,12 @@ void AvatarBookmarks::addBookmark(const QString& bookmarkName) { } void AvatarBookmarks::saveBookmark(const QString& bookmarkName) { + if (ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission::SCRIPT_PERMISSION_GET_AVATAR_URL)) { + saveBookmarkInternal(bookmarkName); + } +} + +void AvatarBookmarks::saveBookmarkInternal(const QString& bookmarkName) { if (QThread::currentThread() != thread()) { BLOCKING_INVOKE_METHOD(this, "saveBookmark", Q_ARG(QString, bookmarkName)); return; @@ -145,6 +166,12 @@ void AvatarBookmarks::saveBookmark(const QString& bookmarkName) { } void AvatarBookmarks::removeBookmark(const QString& bookmarkName) { + if (ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission::SCRIPT_PERMISSION_GET_AVATAR_URL)) { + removeBookmarkInternal(bookmarkName); + } +} + +void AvatarBookmarks::removeBookmarkInternal(const QString& bookmarkName) { if (QThread::currentThread() != thread()) { BLOCKING_INVOKE_METHOD(this, "removeBookmark", Q_ARG(QString, bookmarkName)); return; @@ -200,6 +227,12 @@ void AvatarBookmarks::updateAvatarEntities(const QVariantList &avatarEntities) { */ void AvatarBookmarks::loadBookmark(const QString& bookmarkName) { + if (ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission::SCRIPT_PERMISSION_GET_AVATAR_URL)) { + loadBookmarkInternal(bookmarkName); + } +} + +void AvatarBookmarks::loadBookmarkInternal(const QString& bookmarkName) { if (QThread::currentThread() != thread()) { BLOCKING_INVOKE_METHOD(this, "loadBookmark", Q_ARG(QString, bookmarkName)); return; @@ -268,6 +301,15 @@ void AvatarBookmarks::readFromFile() { } QVariantMap AvatarBookmarks::getBookmark(const QString &bookmarkName) +{ + if (ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission::SCRIPT_PERMISSION_GET_AVATAR_URL)) { + return getBookmarkInternal(bookmarkName); + } else { + return {}; + } +} + +QVariantMap AvatarBookmarks::getBookmarkInternal(const QString &bookmarkName) { if (QThread::currentThread() != thread()) { QVariantMap result; diff --git a/interface/src/AvatarBookmarks.h b/interface/src/AvatarBookmarks.h index c2c7eb5a0a..bf06743b3f 100644 --- a/interface/src/AvatarBookmarks.h +++ b/interface/src/AvatarBookmarks.h @@ -100,7 +100,7 @@ public slots: * print("- " + key + " " + bookmarks[key].avatarUrl); * }; */ - QVariantMap getBookmarks() { return _bookmarks; } + QVariantMap getBookmarks(); signals: /*@jsdoc @@ -147,6 +147,11 @@ protected slots: void deleteBookmark() override; private: + QVariantMap getBookmarkInternal(const QString &bookmarkName); + void addBookmarkInternal(const QString& bookmarkName); + void saveBookmarkInternal(const QString& bookmarkName); + void loadBookmarkInternal(const QString& bookmarkName); + void removeBookmarkInternal(const QString& bookmarkName); const QString AVATARBOOKMARKS_FILENAME = "avatarbookmarks.json"; const QString ENTRY_AVATAR_URL = "avatarUrl"; const QString ENTRY_AVATAR_ICON = "avatarIcon"; diff --git a/libraries/script-engine/src/ScriptPermissions.cpp b/libraries/script-engine/src/ScriptPermissions.cpp index 80bdb5ef4d..91d225b98f 100644 --- a/libraries/script-engine/src/ScriptPermissions.cpp +++ b/libraries/script-engine/src/ScriptPermissions.cpp @@ -46,8 +46,12 @@ bool ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission per // Get the script manager: auto engine = Scriptable::engine(); if (!engine) { - qDebug() << "ScriptPermissions::isCurrentScriptAllowed called outside script engine for permission: " << scriptPermissionNames[permissionIndex]; - return false; + // When this happens it means that function was called from QML or C++ and should always be allowed + if (PERMISSIONS_DEBUG_ENABLED) { + qDebug() << "ScriptPermissions::isCurrentScriptAllowed called outside script engine for permission: " + << scriptPermissionNames[permissionIndex]; + } + return true; } auto manager = engine->manager(); if (!manager) { @@ -76,7 +80,7 @@ bool ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission per } // Check if the script is allowed: QList safeURLPrefixes = { "file:///", "qrc:/", NetworkingConstants::OVERTE_COMMUNITY_APPLICATIONS, - NetworkingConstants::OVERTE_TUTORIAL_SCRIPTS/*, "about:console"*/}; + NetworkingConstants::OVERTE_TUTORIAL_SCRIPTS, "about:console"}; Setting::Handle allowedURLsSetting(scriptPermissionSettingKeyNames[permissionIndex]); QList allowedURLs = allowedURLsSetting.get().split("\n"); From 5f0f03af0a11b000d86a5a142e6295b41ed4c0a1 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Tue, 26 Mar 2024 00:27:58 +0100 Subject: [PATCH 4/9] Fixed private settings bug --- interface/src/scripting/SettingsScriptingInterface.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/src/scripting/SettingsScriptingInterface.cpp b/interface/src/scripting/SettingsScriptingInterface.cpp index cf3737cd9c..f12e40c1e8 100644 --- a/interface/src/scripting/SettingsScriptingInterface.cpp +++ b/interface/src/scripting/SettingsScriptingInterface.cpp @@ -25,7 +25,7 @@ QVariant SettingsScriptingInterface::getValue(const QString& setting) { if (!value.isValid()) { value = ""; } - if (_restrictPrivateValues || setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { + if (_restrictPrivateValues && setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { value = ""; } return value; @@ -36,7 +36,7 @@ QVariant SettingsScriptingInterface::getValue(const QString& setting, const QVar if (!value.isValid()) { value = ""; } - if (_restrictPrivateValues || setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { + if (_restrictPrivateValues && setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { value = ""; } return value; From 379db8b17c45b4265670d67ab3f8482924ff18bf Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sun, 14 Apr 2024 18:11:27 +0200 Subject: [PATCH 5/9] Simplify AvatarData::getSkeletonModelURLFromScript() Co-authored-by: HifiExperiments <53453710+HifiExperiments@users.noreply.github.com> --- libraries/avatars/src/AvatarData.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index da48bced06..21cfec65d4 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2107,15 +2107,11 @@ const QUrl& AvatarData::getSkeletonModelURL() const { } QString AvatarData::getSkeletonModelURLFromScript() const { - if (isMyAvatar()) { - if (!isMyAvatarURLProtected()) { - return _skeletonModelURL.toString(); - } else { - return QString(); - } - } else { - return QString(); + if (isMyAvatar() && !isMyAvatarURLProtected()) { + return _skeletonModelURL.toString(); } + + return QString(); }; QByteArray AvatarData::packSkeletonData() const { From e57874a2bd86512fa1019b815a2a546ed1902b95 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sun, 14 Apr 2024 18:12:30 +0200 Subject: [PATCH 6/9] Simplify ScriptAvatarData::getSkeletonModelURLFromScript Co-authored-by: HifiExperiments <53453710+HifiExperiments@users.noreply.github.com> --- libraries/avatars/src/ScriptAvatarData.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/libraries/avatars/src/ScriptAvatarData.cpp b/libraries/avatars/src/ScriptAvatarData.cpp index 5a2cd2d225..3b5397fd55 100644 --- a/libraries/avatars/src/ScriptAvatarData.cpp +++ b/libraries/avatars/src/ScriptAvatarData.cpp @@ -204,15 +204,11 @@ bool ScriptAvatarData::getLookAtSnappingEnabled() const { // QString ScriptAvatarData::getSkeletonModelURLFromScript() const { if (AvatarSharedPointer sharedAvatarData = _avatarData.lock()) { - if (sharedAvatarData->isMyAvatar()) { - if (sharedAvatarData->isMyAvatarURLProtected()) { - return QString(); - } else { - return sharedAvatarData->getSkeletonModelURLFromScript(); - } - } else { - return QString(); + if (sharedAvatarData->isMyAvatar() && !sharedAvatarData->isMyAvatarURLProtected()) { + return sharedAvatarData->getSkeletonModelURLFromScript(); } + + return QString(); } else { return QString(); } From 1887a82b4b0938562432a1e5d8cd6e675bc4de04 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sun, 14 Apr 2024 20:22:22 +0200 Subject: [PATCH 7/9] Script security fixes and cleanups --- .../hifi/dialogs/security/ScriptSecurity.qml | 32 ++--------- interface/src/CrashRecoveryHandler.cpp | 2 - .../scripting/SettingsScriptingInterface.cpp | 12 ++-- libraries/avatars/src/AvatarData.h | 2 + .../script-engine/src/ScriptPermissions.cpp | 55 ++++++++++++------- libraries/shared/src/SpatiallyNestable.h | 1 - 6 files changed, 49 insertions(+), 55 deletions(-) diff --git a/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml b/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml index b36872d387..fa8f02d6df 100644 --- a/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml +++ b/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml @@ -29,44 +29,24 @@ Rectangle { function getWhitelistAsText() { var whitelist = Settings.getValue("private/scriptPermissionGetAvatarURLSafeURLs"); - var arrayWhitelist = whitelist.split(",").join("\n"); + var arrayWhitelist = whitelist.replace(",", "\n"); return arrayWhitelist; } function setWhitelistAsText(whitelistText) { Settings.setValue("private/scriptPermissionGetAvatarURLSafeURLs", whitelistText.text); - - var originalSetString = whitelistText.text; - var originalSet = originalSetString.split(' ').join(''); - - var check = Settings.getValue("private/scriptPermissionGetAvatarURLSafeURLs"); - var arrayCheck = check.split(",").join("\n"); - - setWhitelistSuccess(arrayCheck === originalSet); + notificationText.text = "Whitelist saved."; } - function setWhitelistSuccess(success) { - if (success) { - notificationText.text = "Successfully saved settings."; - } else { - notificationText.text = "Error! Settings not saved."; - } - } - - function toggleWhitelist(enabled) { + function setAvatarProtection(enabled) { Settings.setValue("private/scriptPermissionGetAvatarURLEnable", enabled); - console.info("Toggling Protect Avatar URLs to:", enabled); + console.info("Setting Protect Avatar URLs to:", enabled); } function initCheckbox() { - var check = Settings.getValue("private/scriptPermissionGetAvatarURLEnable", true); - - if (check) { - whitelistEnabled.toggle(); - } + whitelistEnabled.checked = Settings.getValue("private/scriptPermissionGetAvatarURLEnable", true); } - anchors.fill: parent width: parent.width; height: 120; @@ -99,7 +79,7 @@ Rectangle { anchors.top: parent.top; anchors.topMargin: 10; onToggled: { - toggleWhitelist(whitelistEnabled.checked) + setAvatarProtection(whitelistEnabled.checked) } Label { diff --git a/interface/src/CrashRecoveryHandler.cpp b/interface/src/CrashRecoveryHandler.cpp index 97e03f003d..c03e8bc70f 100644 --- a/interface/src/CrashRecoveryHandler.cpp +++ b/interface/src/CrashRecoveryHandler.cpp @@ -281,7 +281,6 @@ void CrashRecoveryHandler::handleCrash(CrashRecoveryHandler::Action action) { // Display name and avatar settings.beginGroup(AVATAR_GROUP); settings.setValue(DISPLAY_NAME_KEY, displayName); - settings.setValue(FULL_AVATAR_URL_KEY, fullAvatarURL); settings.setValue(FULL_AVATAR_MODEL_NAME_KEY, fullAvatarModelName); settings.endGroup(); @@ -291,4 +290,3 @@ void CrashRecoveryHandler::handleCrash(CrashRecoveryHandler::Action action) { settings.setValue(TUTORIAL_COMPLETE_FLAG_KEY, tutorialComplete); } } - diff --git a/interface/src/scripting/SettingsScriptingInterface.cpp b/interface/src/scripting/SettingsScriptingInterface.cpp index f12e40c1e8..00cdf009eb 100644 --- a/interface/src/scripting/SettingsScriptingInterface.cpp +++ b/interface/src/scripting/SettingsScriptingInterface.cpp @@ -21,24 +21,24 @@ SettingsScriptingInterface* SettingsScriptingInterface::getInstance() { } QVariant SettingsScriptingInterface::getValue(const QString& setting) { + if (_restrictPrivateValues && setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { + return {""}; + } QVariant value = Setting::Handle(setting).get(); if (!value.isValid()) { value = ""; } - if (_restrictPrivateValues && setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { - value = ""; - } return value; } QVariant SettingsScriptingInterface::getValue(const QString& setting, const QVariant& defaultValue) { + if (_restrictPrivateValues && setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { + return {""}; + } QVariant value = Setting::Handle(setting, defaultValue).get(); if (!value.isValid()) { value = ""; } - if (_restrictPrivateValues && setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) { - value = ""; - } return value; } diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 69dd747543..d3bf8a3282 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -610,6 +610,8 @@ public: AvatarData(); virtual ~AvatarData(); + virtual bool isMyAvatarURLProtected() const { return false; } // This needs to be here because both MyAvatar and AvatarData inherit from MyAvatar + static const QUrl& defaultFullAvatarModelUrl(); const QUuid getSessionUUID() const { return getID(); } diff --git a/libraries/script-engine/src/ScriptPermissions.cpp b/libraries/script-engine/src/ScriptPermissions.cpp index 91d225b98f..8012f198f6 100644 --- a/libraries/script-engine/src/ScriptPermissions.cpp +++ b/libraries/script-engine/src/ScriptPermissions.cpp @@ -60,24 +60,32 @@ bool ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission per } std::vector urlsToCheck; QString scriptURL = manager->getAbsoluteFilename(); - if (scriptURL.startsWith("about:Entities")) { - // This is entity script manager, we need to find the file name of the current script instead - scriptURL = Scriptable::context()->currentFileName(); + + // If this is an entity script manager, we need to find the file name of the current script instead + if (!scriptURL.startsWith("about:Entities")) { urlsToCheck.push_back(scriptURL); - if (PERMISSIONS_DEBUG_ENABLED) { - qDebug() << "ScriptPermissions::isCurrentScriptAllowed: filename: " << scriptURL; - } - auto parentContext = Scriptable::context()->parentContext(); - while (parentContext) { + } + + auto currentURL = Scriptable::context()->currentFileName(); + if (!currentURL.isEmpty() && currentURL != scriptURL) { + urlsToCheck.push_back(currentURL); + } + + if (PERMISSIONS_DEBUG_ENABLED) { + qDebug() << "ScriptPermissions::isCurrentScriptAllowed: filename: " << scriptURL; + } + auto parentContext = Scriptable::context()->parentContext(); + while (parentContext) { + QString parentFilename = parentContext->currentFileName(); + if (!parentFilename.isEmpty()) { urlsToCheck.push_back(parentContext->currentFileName()); if (PERMISSIONS_DEBUG_ENABLED) { qDebug() << "ScriptPermissions::isCurrentScriptAllowed: parent filename: " << parentContext->currentFileName(); } - parentContext = parentContext->parentContext(); } - } else { - urlsToCheck.push_back(scriptURL); + parentContext = parentContext->parentContext(); } + // Check if the script is allowed: QList safeURLPrefixes = { "file:///", "qrc:/", NetworkingConstants::OVERTE_COMMUNITY_APPLICATIONS, NetworkingConstants::OVERTE_TUTORIAL_SCRIPTS, "about:console"}; @@ -88,19 +96,26 @@ bool ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission per safeURLPrefixes.push_back(entry); } - for (const auto& str : safeURLPrefixes) { - if (!str.isEmpty() && scriptURL.startsWith(str)) { + for (auto urlToCheck : urlsToCheck) { + bool urlIsAllowed = false; + for (const auto& str : safeURLPrefixes) { + if (!str.isEmpty() && urlToCheck.startsWith(str)) { + urlIsAllowed = true; + if (PERMISSIONS_DEBUG_ENABLED) { + qDebug() << "ScriptPermissions::isCurrentScriptAllowed: " << scriptPermissionNames[permissionIndex] + << " for script " << urlToCheck << " accepted with rule: " << str; + } + } + } + + if (!urlIsAllowed) { if (PERMISSIONS_DEBUG_ENABLED) { qDebug() << "ScriptPermissions::isCurrentScriptAllowed: " << scriptPermissionNames[permissionIndex] - << " for script " << scriptURL << " accepted with rule: " << str; + << " for script " << urlToCheck << " rejected."; } - return true; + return false; } } - if (PERMISSIONS_DEBUG_ENABLED) { - qDebug() << "ScriptPermissions::isCurrentScriptAllowed: " << scriptPermissionNames[permissionIndex] << " for script " - << scriptURL << " rejected."; - } - return false; + return true; } \ No newline at end of file diff --git a/libraries/shared/src/SpatiallyNestable.h b/libraries/shared/src/SpatiallyNestable.h index 5de86d7f95..29f23afdfb 100644 --- a/libraries/shared/src/SpatiallyNestable.h +++ b/libraries/shared/src/SpatiallyNestable.h @@ -47,7 +47,6 @@ public: virtual void setParentID(const QUuid& parentID); virtual bool isMyAvatar() const { return false; } - virtual bool isMyAvatarURLProtected() const { return false; } // This needs to be here because both MyAvatar and AvatarData inherit from MyAvatar virtual quint16 getParentJointIndex() const { return _parentJointIndex; } virtual void setParentJointIndex(quint16 parentJointIndex); From 95406c44c687f4afd1c74d5557fdee99d4e92fa7 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sun, 14 Apr 2024 21:56:41 +0200 Subject: [PATCH 8/9] Simplified ScriptSecurity.qml --- .../qml/hifi/dialogs/security/ScriptSecurity.qml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml b/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml index fa8f02d6df..de7304b6fb 100644 --- a/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml +++ b/interface/resources/qml/hifi/dialogs/security/ScriptSecurity.qml @@ -43,10 +43,6 @@ Rectangle { console.info("Setting Protect Avatar URLs to:", enabled); } - function initCheckbox() { - whitelistEnabled.checked = Settings.getValue("private/scriptPermissionGetAvatarURLEnable", true); - } - anchors.fill: parent width: parent.width; height: 120; @@ -69,12 +65,10 @@ Rectangle { height: 60; CheckBox { - Component.onCompleted: { - initCheckbox(); - } - id: whitelistEnabled; + checked: Settings.getValue("private/scriptPermissionGetAvatarURLEnable", true); + anchors.right: parent.right; anchors.top: parent.top; anchors.topMargin: 10; From 5aac93352c54176099c836873ee4608eb1c430e4 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Wed, 17 Apr 2024 00:45:57 +0200 Subject: [PATCH 9/9] Fixed check for if script permission is enabled --- libraries/script-engine/src/ScriptPermissions.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/script-engine/src/ScriptPermissions.cpp b/libraries/script-engine/src/ScriptPermissions.cpp index 8012f198f6..a817f00056 100644 --- a/libraries/script-engine/src/ScriptPermissions.cpp +++ b/libraries/script-engine/src/ScriptPermissions.cpp @@ -43,6 +43,9 @@ bool ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission per int permissionIndex = static_cast(permission); // Check if the permission checking is active Setting::Handle isCheckingEnabled(scriptPermissionSettingEnableKeyNames[permissionIndex], scriptPermissionSettingEnableDefaultValues[permissionIndex]); + if (!isCheckingEnabled.get()) { + return true; + } // Get the script manager: auto engine = Scriptable::engine(); if (!engine) {