Updates

Author: cmdcolin

src/JBrowse/Browser.js

@@ -2179,7 +2179,13 @@ define([
         if (accounts && !lang.isArray(userAccounts)) {
           userAccounts = userAccounts.replace(/^\s*|\s*$/, '').split(/\s*,\s*/)
         }
-        accounts.push.apply(accounts, userAccounts)
+        // validate GA account IDs to prevent script injection (UA-XXXXX-X or G-XXXXXXXXXX format)
+        var gaPattern = /^(UA-\d+-\d+|G-[A-Z0-9]+)$/i
+        for (var i = 0; i < userAccounts.length; i++) {
+          if (gaPattern.test(userAccounts[i])) {
+            accounts.push(userAccounts[i])
+          }
+        }
       }
 
       var analyticsScript =

src/JBrowse/ConfigAdaptor/AdaptorUtil.js

@@ -27,7 +27,10 @@ define([
         return arguments[0]
       }
       try {
-        eval(`arguments[0]=${arguments[0]};`)
+        // use Function constructor instead of eval for slightly better isolation
+        // note: this still executes untrusted code, but doesn't have access to local scope
+        var fn = new Function(`return (${arguments[0]});`)
+        return fn()
       } catch (e) {
         console.error(`${e} parsing config callback '${arguments[0]}'`)
       }

src/JBrowse/Util.js

@@ -38,6 +38,43 @@ define([
       return dompurify.sanitize(str)
     },
 
+    isSafeUrl: function (url) {
+      if (!url || typeof url !== 'string') {
+        return false
+      }
+      var trimmed = url.trim().toLowerCase()
+      // block dangerous URL schemes
+      if (
+        trimmed.startsWith('javascript:') ||
+        trimmed.startsWith('data:') ||
+        trimmed.startsWith('vbscript:')
+      ) {
+        return false
+      }
+      return true
+    },
+
+    sanitizeCss: function (css) {
+      if (!css || typeof css !== 'string') {
+        return ''
+      }
+      // remove dangerous CSS patterns: url(), expression(), @import, behavior, -moz-binding
+      return css
+        .replace(/url\s*\([^)]*\)/gi, '')
+        .replace(/expression\s*\([^)]*\)/gi, '')
+        .replace(/@import[^;]*/gi, '')
+        .replace(/behavior\s*:[^;]*/gi, '')
+        .replace(/-moz-binding\s*:[^;]*/gi, '')
+    },
+
+    sanitizeClassName: function (className) {
+      if (!className || typeof className !== 'string') {
+        return ''
+      }
+      // only allow alphanumeric, hyphens, underscores, and spaces (for multiple classes)
+      return className.replace(/[^a-zA-Z0-9_\- ]/g, '')
+    },
+
     unescapeHTML: function (str) {
       return str
         .toString()

src/JBrowse/View/Track/BlockBased.js

@@ -1015,9 +1015,13 @@ define([
             }[`${spec.action}`.toLowerCase()]
 
             if (spec.action == 'newWindow') {
-              window.open(url, '_blank')
+              if (Util.isSafeUrl(url)) {
+                window.open(url, '_blank')
+              }
             } else if (spec.action == 'navigateTo') {
-              window.location = url
+              if (Util.isSafeUrl(url)) {
+                window.location = url
+              }
             } else if (
               spec.action in
               {
@@ -1321,11 +1325,11 @@ define([
             width: iframeDims.w,
             height: iframeDims.h,
             style: { border: 'none' },
-            src: spec.url,
+            src: Util.isSafeUrl(spec.url) ? spec.url : 'about:blank',
           })
 
           dialog.set('content', iframe)
-          if (!spec.hideIframeDialogUrl) {
+          if (!spec.hideIframeDialogUrl && Util.isSafeUrl(spec.url)) {
             dojo.create(
               'a',
               {

src/JBrowse/View/Track/HTMLFeatures.js

@@ -1117,11 +1117,10 @@ define([
         var glyphBox
         heightTest = document.createElement('div')
         //cover all the bases: stranded or not, phase or not
-        heightTest.className = `feature ${this.config.style.className} plus-${
-          this.config.style.className
-        } plus-${this.config.style.className}1`
+        var safeClassName = Util.sanitizeClassName(this.config.style.className)
+        heightTest.className = `feature ${safeClassName} plus-${safeClassName} plus-${safeClassName}1`
         if (this.config.style.featureCss) {
-          heightTest.style.cssText = this.config.style.featureCss
+          heightTest.style.cssText = Util.sanitizeCss(this.config.style.featureCss)
         }
         heightTest.style.visibility = 'hidden'
         if (Util.is_ie6) {
@@ -1338,7 +1337,9 @@ define([
         dojo.addClass(featDiv, 'feature')
         var className = this.config.style.className
         if (className == '{type}') {
-          className = feature.get('type')
+          className = Util.sanitizeClassName(feature.get('type'))
+        } else {
+          className = Util.sanitizeClassName(className)
         }
         var strand = feature.get('strand')
         switch (strand) {
@@ -1386,7 +1387,7 @@ define([
           `left:${(100 * (displayStart - block.startBase)) / blockWidth}%;` +
           `top:${top}px;` +
           ` width:${featwidth}%;${
-            this.config.style.featureCss ? this.config.style.featureCss : ''
+            this.config.style.featureCss ? Util.sanitizeCss(this.config.style.featureCss) : ''
           }`
 
         // Store the containerStart/End so we can resolve the truncation
@@ -1398,16 +1399,17 @@ define([
           var ah = document.createElement('div')
           var featwidth_px = (featwidth / 100) * blockWidth * scale
 
+          var safeArrowClass = Util.sanitizeClassName(this.config.style.arrowheadClass)
           switch (strand) {
             case 1:
             case '+':
-              ah.className = `plus-${this.config.style.arrowheadClass}`
+              ah.className = `plus-${safeArrowClass}`
               ah.style.cssText = `right: ${-this.plusArrowWidth}px`
               featDiv.appendChild(ah)
               break
             case -1:
             case '-':
-              ah.className = `minus-${this.config.style.arrowheadClass}`
+              ah.className = `minus-${safeArrowClass}`
               ah.style.cssText = `left: ${-this.minusArrowWidth}px`
               featDiv.appendChild(ah)
               break