Script security fixes and cleanups

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);
-
+        notificationText.text = "Whitelist saved.";
-        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) {
+    function setAvatarProtection(enabled) {
-        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);
+        console.info("Setting Protect Avatar URLs to:", enabled);
     }
 
     function initCheckbox() {
-        var check = Settings.getValue("private/scriptPermissionGetAvatarURLEnable", true);
+        whitelistEnabled.checked = Settings.getValue("private/scriptPermissionGetAvatarURLEnable", true);
-
-        if (check) {
-            whitelistEnabled.toggle();
-        }
     }
   
-  
     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 {

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);
     }
 }
-

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<QVariant>(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<QVariant>(setting, defaultValue).get();
     if (!value.isValid()) {
         value = "";
     }
-    if (_restrictPrivateValues && setting.startsWith(SETTINGS_FULL_PRIVATE_GROUP_NAME + "/")) {
-        value = "";
-    }
     return value;
 }
 

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(); }

libraries/script-engine/src/ScriptPermissions.cpp

@@ -60,24 +60,32 @@ bool ScriptPermissions::isCurrentScriptAllowed(ScriptPermissions::Permission per
     }
     std::vector<QString> 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
+    // If this is an entity script manager, we need to find the file name of the current script instead
-        scriptURL = Scriptable::context()->currentFileName();
+    if (!scriptURL.startsWith("about:Entities")) {
         urlsToCheck.push_back(scriptURL);
-        if (PERMISSIONS_DEBUG_ENABLED) {
+    }
-            qDebug() << "ScriptPermissions::isCurrentScriptAllowed: filename: " << scriptURL;
+
-        }
+    auto currentURL = Scriptable::context()->currentFileName();
-        auto parentContext = Scriptable::context()->parentContext();
+    if (!currentURL.isEmpty() && currentURL != scriptURL) {
-        while (parentContext) {
+        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 {
+        parentContext = parentContext->parentContext();
-        urlsToCheck.push_back(scriptURL);
     }
+
     // Check if the script is allowed:
     QList<QString> 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) {
+    for (auto urlToCheck : urlsToCheck) {
-        if (!str.isEmpty() && scriptURL.startsWith(str)) {
+        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) {
+    return true;
-        qDebug() << "ScriptPermissions::isCurrentScriptAllowed: " << scriptPermissionNames[permissionIndex] << " for script "
-                 << scriptURL << " rejected.";
-    }
-    return false;
 }

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);