6c1b7e8cb87b181f3220efc0ed86dbb1ceea713e
braney
  Thu Apr 16 10:36:16 2026 -0700
hgTracks: harden Change Track Color dialog against XSS and invalid input, refs #20460

In response to findings from the nightly code review:
inject hub-controlled shortLabel via .text() instead of HTML concatenation,
validate the free-text color input with a hex regex before writing to the
cart, and fold the duplicated Apply/Ok handlers into a shared closure.

diff --git src/hg/js/hgTracks.js src/hg/js/hgTracks.js
index aa2e4624c7f..20c825d4e0e 100644
--- src/hg/js/hgTracks.js
+++ src/hg/js/hgTracks.js
@@ -2761,87 +2761,91 @@
         // seen in FireFox 3.X, which occurred b/c FF doesn't actually fetch the image until
         // the menu is being shown.
         return "<img style='width:16px; height:16px; border-style:none;' src='../images/" +
                 img + "' />";
     },
 
     showColorPicker: function (trackName)
     {   // Show a small dialog with a spectrum color picker for changing track color
         var rec = hgTracks.trackDb[trackName];
         if (!rec || !rec.defaultColor)
             return;
         var currentColor = (rec.colorOverrideOn && rec.colorOverride) ?
                            rec.colorOverride : rec.defaultColor;
         var dialogId = "trackColorDialog";
         $("#" + dialogId).remove();
-        $("body").append(
-            "<div id='" + dialogId + "'>" +
-            "<p>Pick a new color for <b>" + rec.shortLabel + "</b>:</p>" +
-            "<input type='text' id='trackColorText' value='" + currentColor + "' size='8' />" +
+        // Build with static template + data injected via .text()/.val()/.prop() so that
+        // hub-provided shortLabel cannot inject HTML.
+        var $dlg = $("<div>").attr("id", dialogId).html(
+            "<p>Pick a new color for <b></b>:</p>" +
+            "<input type='text' id='trackColorText' size='8' />" +
             "&nbsp;<input id='trackColorPicker' />" +
-            "<br><br><label><input type='checkbox' id='trackColorOn'" +
-            (rec.colorOverrideOn ? " checked" : "") +
-            " /> Enable color override</label>" +
-            "</div>");
+            "<br><br><label><input type='checkbox' id='trackColorOn' /> " +
+            "Enable color override</label>");
+        $dlg.find("p b").text(rec.shortLabel);
+        $dlg.find("#trackColorText").val(currentColor);
+        $dlg.find("#trackColorOn").prop("checked", !!rec.colorOverrideOn);
+        $("body").append($dlg);
+        var hexColorRe = /^#[0-9a-fA-F]{6}$/;
         $("#trackColorPicker").spectrum({
             color: currentColor,
             showPalette: true,
             showSelectionPalette: true,
             showInitial: true,
             showInput: true,
             preferredFormat: "hex",
             localStorageKey: "genomebrowser",
             hideAfterPaletteSelect: true,
             change: function(color) {
                 $("#trackColorText").val(color.toHexString());
                 $("#trackColorOn").prop("checked", true);
             }
         });
         $("#trackColorText").on("change", function() {
-            $("#trackColorPicker").spectrum("set", $(this).val());
+            var val = $(this).val();
+            if (!hexColorRe.test(val))
+                return;
+            $("#trackColorPicker").spectrum("set", val);
             $("#trackColorOn").prop("checked", true);
         });
-        $("#" + dialogId).dialog({
-            modal: true,
-            title: "Change Track Color",
-            closeOnEscape: true,
-            resizable: false,
-            minWidth: 400,
-            buttons: {
-                "Apply": function() {
+        var applyColor = function() {
             var color = $("#trackColorText").val();
+            if (!hexColorRe.test(color)) {
+                warn("Invalid color '" + color + "'. Expected hex format like #1a2b3c.");
+                return false;
+            }
             var isOn = $("#trackColorOn").is(":checked") ? "1" : "0";
             rec.colorOverride = color;
             rec.colorOverrideOn = (isOn === "1");
             cart.setVars(
                 [trackName + ".colorOverride", trackName + ".colorOverrideOn"],
                 [color, isOn], null, false);
             imageV2.requestImgUpdate(trackName,
                 trackName + ".colorOverride=" + encodeURIComponent(color) +
                 "&" + trackName + ".colorOverrideOn=" + isOn);
-                },
+            return true;
+        };
+        $("#" + dialogId).dialog({
+            modal: true,
+            title: "Change Track Color",
+            closeOnEscape: true,
+            resizable: false,
+            minWidth: 400,
+            buttons: {
+                "Apply": function() { applyColor(); },
                 "Ok": function() {
-                    var color = $("#trackColorText").val();
-                    var isOn = $("#trackColorOn").is(":checked") ? "1" : "0";
-                    rec.colorOverride = color;
-                    rec.colorOverrideOn = (isOn === "1");
-                    cart.setVars(
-                        [trackName + ".colorOverride", trackName + ".colorOverrideOn"],
-                        [color, isOn], null, false);
-                    imageV2.requestImgUpdate(trackName,
-                        trackName + ".colorOverride=" + encodeURIComponent(color) +
-                        "&" + trackName + ".colorOverrideOn=" + isOn);
+                    if (applyColor())
                         $(this).dialog("close");
                 }
             },
             close: function() {
                 $("#trackColorPicker").spectrum("destroy");
                 $(this).remove();
             }
         });
     },
 
     // CGIs now use HTML tags, e.g. "<b>Transcript:</b> ENST00000297261.7<br><b>Strand:</b>"
     mouseOverToLabel: function(title)
     {
         if (title.search(/<b>Transcript: ?<[/]b>/) !== -1) {
             title = title.split("<br>")[0].split("</b>")[1];