Change newtab default, minor DOMPurify setup refactor

candela97 · Aug 4, 2019 · a4f5f3d · a4f5f3d
1 parent 92df392
commit a4f5f3d
Showing 1 changed file with 35 additions and 23 deletions.
diff --git a/js/core.js b/js/core.js
@@ -538,7 +538,7 @@ SyncedStorage.defaults = {
     'skip_got_steam': false,
 
     'hideaboutlinks': false,
-    'openinnewtab': true,
+    'openinnewtab': false,
     'keepssachecked': false,
     'showemptywishlist': true,
     'showusernotes': true,
@@ -614,39 +614,51 @@ class ExtensionResources {
 }
 
 /**
- * Allow links to own extension (e.g. options.html)
+ * DOMPurify setup
  * @see https://github.com/cure53/DOMPurify
- * Default RegExp: /^(?:(?:(?:f|ht)tps?|mailto|tel|callto|cid|xmpp):|[^a-z]|[a-z+.\-]+(?:[^a-z+.\-:]|$))/i;
  */
-(function() {
+(async function() {
+    let allowOpenInNewTab = SyncedStorage.defaults.openinnewtab;
+    try {
+        await SyncedStorage;
+        allowOpenInNewTab = SyncedStorage.get("openinnewtab");
+    } catch(e) {
+        console.error(e);
+    }
+
     /**
      * NOTE FOR ADDON REVIEWER:
-     * We are modifying default DOMPurify settings to to allow other protocols in URLs.
+     * We are modifying default DOMPurify settings to allow other protocols in URLs
+     * and to allow links to safely open in new tabs.
+     *
      * We took the original Regex and aded chrome-extension://, moz-extension:// and steam://
      * First two are needed for linking local resources from extension,
      * steam:// protocol is used by Steam store to open their own client (e.g. when you want to launch a game).
      * 
-     * The addition of the "target" attribute to the allowed attributes is done in order to be able to open links in a new tab,
-     * while considering security concerns (see hook and https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/)
+     * The addition of the `target` attribute to the allowed attributes is done in order to be able to open links in a new tab.
+     * We only allow target="_blank" while adding rel="noreferrer noopener" to prevent child window to access window.opener
+     * as described in https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/
      */
-    let purifyConfig = { ALLOWED_URI_REGEXP: /^(?:(?:(?:f|ht)tps?|mailto|tel|callto|cid|xmpp|chrome-extension|moz-extension|steam):|[^a-z]|[a-z+.\-]+(?:[^a-z+.\-:]|$))/i };
-    SyncedStorage.then(() => {
-        if (SyncedStorage.get("openinnewtab")) {
-            purifyConfig.ADD_ATTR = ["target"];
-
-            DOMPurify.addHook("uponSanitizeAttribute", (node, data) => {
-                if (data.attrName === "target") {
-                    if (data.attrValue === "_blank") {
-                        node.setAttribute("rel", "noreferrer noopener");
-                    } else {
-                        data.keepAttr = false;
-                    }
+
+    let purifyConfig = {
+        ALLOWED_URI_REGEXP: /^(?:(?:(?:f|ht)tps?|mailto|tel|callto|cid|xmpp|chrome-extension|moz-extension|steam):|[^a-z]|[a-z+.\-]+(?:[^a-z+.\-:]|$))/i
+    };
+
+    if (allowOpenInNewTab) {
+        purifyConfig.ADD_ATTR = ["target"];
+
+        DOMPurify.addHook("uponSanitizeAttribute", (node, data) => {
+            if (data.attrName === "target") {
+                if (data.attrValue === "_blank") {
+                    node.setAttribute("rel", "noreferrer noopener");
+                } else {
+                    data.keepAttr = false;
                 }
-            });
-        }
+            }
+        });
+    }
 
-        DOMPurify.setConfig(purifyConfig);
-    }, err => console.error(err));
+    DOMPurify.setConfig(purifyConfig);
 })();
 
 class HTML {