Use DOMPurify to sanitize all possible HTML render paths

brad-richardson · brad-richardson · commit 4aa3c865692c · 2025-12-10T12:16:41.000-05:00
More fixes
diff --git a/modules/services/EsriService.js b/modules/services/EsriService.js
@@ -5,7 +5,7 @@ import { utilQsString } from '@rapid-sdk/util';
 import { AbstractSystem } from '../core/AbstractSystem.js';
 import { Graph, Tree, RapidDataset } from '../core/lib/index.js';
 import { osmNode, osmRelation, osmWay } from '../osm/index.js';
-import { utilFetchResponse } from '../util/index.js';
+import { utilFetchResponse, utilSanitizeHTML } from '../util/index.js';
 
 
 const GROUPID = 'bdf6c800b3ae453b9db239e03d7c1727';
@@ -281,13 +281,13 @@ export class EsriService extends AbstractSystem {
     ds.lastv = null;
     ds.layer = null;   // the schema info will live here
 
-    // Cleanup the `licenseInfo` field by removing styles  (not used currently)
+    // Cleanup and sanitize the `licenseInfo` field from Esri API
     const license = select(document.createElement('div'));
-    license.html(ds.licenseInfo);       // set innerHtml
+    license.html(utilSanitizeHTML(ds.licenseInfo));  // sanitize then set innerHTML
     license.selectAll('*')
       .attr('style', null)
       .attr('size', null);
-    ds.license_html = license.html();   // get innerHtml
+    ds.license_html = license.html();   // get innerHTML
   }
 
 
diff --git a/modules/services/KeepRightService.js b/modules/services/KeepRightService.js
@@ -4,7 +4,7 @@ import RBush from 'rbush';
 
 import { AbstractSystem } from '../core/AbstractSystem.js';
 import { QAItem } from '../osm/qa_item.js';
-import { utilFetchResponse } from '../util/index.js';
+import { utilFetchResponse, utilSanitizeHTML } from '../util/index.js';
 
 
 const KEEPRIGHT_API = 'https://www.keepright.at';
@@ -470,8 +470,8 @@ export class KeepRightService extends AbstractSystem {
       idType = 'IDs' in issueTemplate ? issueTemplate.IDs[i-1] : '';
       if (idType && capture) {   // link IDs if present in the capture
         capture = this._parseError(capture, idType);
-      } else if (htmlRegex.test(capture)) {   // escape any html in non-IDs
-        capture = '\\' +  capture + '\\';
+      } else if (htmlRegex.test(capture)) {   // sanitize any html in non-IDs
+        capture = utilSanitizeHTML(capture);
       } else {
         const compare = capture.toLowerCase();
         if (this._krData.localizeStrings[compare]) {   // some replacement strings can be localized
diff --git a/modules/services/OsmoseService.js b/modules/services/OsmoseService.js
@@ -6,7 +6,7 @@ import RBush from 'rbush';
 
 import { AbstractSystem } from '../core/AbstractSystem.js';
 import { QAItem } from '../osm/qa_item.js';
-import { utilFetchResponse } from '../util/index.js';
+import { utilFetchResponse, utilSanitizeHTML } from '../util/index.js';
 
 
 const TILEZOOM = 14;
@@ -198,7 +198,7 @@ export class OsmoseService extends AbstractSystem {
       // Assign directly for immediate use in the callback
       issue.elems = data.elems.map(e => e.type.substring(0,1) + e.id);
       // Some issues have instance specific detail in a subtitle
-      issue.detail = data.subtitle ? marked.parse(data.subtitle.auto) : '';
+      issue.detail = data.subtitle ? utilSanitizeHTML(marked.parse(data.subtitle.auto)) : '';
       this.replaceItem(issue);
     };
 
@@ -433,9 +433,9 @@ export class OsmoseService extends AbstractSystem {
         let issueStrings = {};
         // Force title to begin with an uppercase letter
         if (title)  issueStrings.title = title.auto.charAt(0).toUpperCase() + title.auto.slice(1);
-        if (detail) issueStrings.detail = marked.parse(detail.auto);
-        if (trap)   issueStrings.trap = marked.parse(trap.auto);
-        if (fix)    issueStrings.fix = marked.parse(fix.auto);
+        if (detail) issueStrings.detail = utilSanitizeHTML(marked.parse(detail.auto));
+        if (trap)   issueStrings.trap = utilSanitizeHTML(marked.parse(trap.auto));
+        if (fix)    issueStrings.fix = utilSanitizeHTML(marked.parse(fix.auto));
 
         stringData[itemType] = issueStrings;
       };
diff --git a/modules/ui/UiAttribution.js b/modules/ui/UiAttribution.js
@@ -1,6 +1,8 @@
 import { selection, select } from 'd3-selection';
 import throttle from 'lodash-es/throttle.js';
 
+import { utilSanitizeHTML } from '../util/index.js';
+
 
 /**
  * UiAttribution
@@ -113,9 +115,9 @@ export class UiAttribution {
       .each((d, i, nodes) => {
         const $$link = select(nodes[i]);
 
-        // add html directly (maybe we shouldn't?)
+        // Sanitize HTML from imagery provider metadata
         if (d.terms_html) {
-          $$link.html(d.terms_html);
+          $$link.html(utilSanitizeHTML(d.terms_html));
           return;
         }
 
diff --git a/modules/ui/UiRapidCatalog.js b/modules/ui/UiRapidCatalog.js
@@ -4,7 +4,7 @@ import { marked } from 'marked';
 
 import { uiIcon } from './icon.js';
 import { uiCombobox} from './combobox.js';
-import { utilKeybinding, utilNoAuto } from '../util/index.js';
+import { utilKeybinding, utilNoAuto, utilSanitizeHTML } from '../util/index.js';
 
 const MAXRESULTS = 100;
 
@@ -563,9 +563,12 @@ export class UiRapidCatalog extends EventEmitter {
 
   /**
    * highlight
+   * Highlights matches of `needle` in `haystack` with <mark> tags.
+   * Sanitizes the haystack first to prevent XSS.
    */
   highlight(needle, haystack) {
-    let html = haystack;
+    // Sanitize the haystack to prevent XSS from dataset labels
+    let html = utilSanitizeHTML(haystack);
     if (needle) {
       const re = new RegExp('\(' + _escapeRegex(needle) + '\)', 'gi');
       html = html.replace(re, '<mark>$1</mark>');
diff --git a/modules/ui/conflicts.js b/modules/ui/conflicts.js
@@ -5,7 +5,7 @@ import { Extent, numWrap } from '@rapid-sdk/math';
 import { JXON } from '../util/jxon.js';
 import { osmChangeset } from '../osm/index.js';
 import { uiIcon } from './icon.js';
-import { utilDetect, utilKeybinding, utilRebind } from '../util/index.js';
+import { utilDetect, utilKeybinding, utilRebind, utilSanitizeHTML } from '../util/index.js';
 
 
 export function uiConflicts(context) {
@@ -183,7 +183,7 @@ export function uiConflicts(context) {
       .enter()
       .append('li')
       .attr('class', 'conflict-detail-item')
-      .html(d => d);
+      .html(d => utilSanitizeHTML(d));
 
     details
       .append('div')
diff --git a/modules/ui/keepRight_details.js b/modules/ui/keepRight_details.js
@@ -1,6 +1,6 @@
 import { select as d3_select } from 'd3-selection';
 
-import { utilHighlightEntities } from '../util/index.js';
+import { utilHighlightEntities, utilSanitizeHTML } from '../util/index.js';
 
 
 export function uiKeepRightDetails(context) {
@@ -52,7 +52,7 @@ export function uiKeepRightDetails(context) {
     descriptionEnter
       .append('div')
       .attr('class', 'qa-details-description-text')
-      .html(issueDetailHTML);
+      .html(d => utilSanitizeHTML(issueDetailHTML(d)));
 
     // If there are entity links in the error message..
     let relatedEntities = [];
diff --git a/modules/ui/maproulette_details.js b/modules/ui/maproulette_details.js
@@ -1,6 +1,6 @@
 import { select as d3_select } from 'd3-selection';
 
-import { utilHighlightEntities } from '../util/index.js';
+import { utilHighlightEntities, utilSanitizeHTML } from '../util/index.js';
 import { marked } from 'marked';
 
 export function uiMapRouletteDetails(context) {
@@ -153,8 +153,8 @@ export function uiMapRouletteDetails(context) {
           .attr('target', '_blank');
       }
 
-      const descriptionHtml = generateDynamicContent(marked.parse(replaceMustacheTags(task.description, task), { async: false }));
-      const instructionHtml = generateDynamicContent(marked.parse(replaceMustacheTags(task.instruction, task), { async: false }));
+      const descriptionHtml = utilSanitizeHTML(generateDynamicContent(marked.parse(replaceMustacheTags(task.description, task), { async: false })));
+      const instructionHtml = utilSanitizeHTML(generateDynamicContent(marked.parse(replaceMustacheTags(task.instruction, task), { async: false })));
 
       // We show the challenge description when user select an unkown challenge.
       // But we hide it if a specific (assumed to be know) challenge is selected.
diff --git a/modules/ui/note_comments.js b/modules/ui/note_comments.js
@@ -1,6 +1,7 @@
 import { select as d3_select } from 'd3-selection';
 
 import { uiIcon } from './icon.js';
+import { utilSanitizeHTML } from '../util/index.js';
 
 
 export function uiNoteComments(context) {
@@ -51,8 +52,11 @@ export function uiNoteComments(context) {
             .attr('href', osm.userURL(d.user))
             .attr('target', '_blank');
         }
-        selection
-          .html(d => d.user || l10n.tHtml('note.anonymous'));
+        if (d.user) {
+          selection.text(d.user);
+        } else {
+          selection.html(l10n.tHtml('note.anonymous'));
+        }
       });
 
     metadataEnter
@@ -63,7 +67,7 @@ export function uiNoteComments(context) {
     mainEnter
       .append('div')
       .attr('class', 'comment-text')
-      .html(d => d.html)
+      .html(d => utilSanitizeHTML(d.html))
       .selectAll('a')
         .attr('rel', 'noopener nofollow')
         .attr('target', '_blank');
diff --git a/modules/ui/success.js b/modules/ui/success.js
@@ -5,6 +5,7 @@ import { resolveStrings } from 'osm-community-index';
 import { uiIcon } from './icon.js';
 import { uiDisclosure } from '../ui/disclosure.js';
 import { utilRebind } from '../util/rebind.js';
+import { utilSanitizeHTML } from '../util/index.js';
 
 
 let _oci = null;
@@ -248,12 +249,12 @@ export function uiSuccess(context) {
     selection
       .append('div')
       .attr('class', 'community-name')
-      .html(d.resolved.nameHTML);
+      .html(utilSanitizeHTML(d.resolved.nameHTML));
 
     selection
       .append('div')
       .attr('class', 'community-description')
-      .html(d.resolved.descriptionHTML);
+      .html(utilSanitizeHTML(d.resolved.descriptionHTML));
 
     // Create an expanding section if any of these are present..
     if (d.resolved.extendedDescriptionHTML || (d.languageCodes && d.languageCodes.length)) {
@@ -310,7 +311,7 @@ export function uiSuccess(context) {
         moreEnter
           .append('div')
           .attr('class', 'community-extended-description')
-          .html(d.resolved.extendedDescriptionHTML);
+          .html(utilSanitizeHTML(d.resolved.extendedDescriptionHTML));
       }
 
       if (d.languageCodes && d.languageCodes.length) {
diff --git a/modules/util/index.js b/modules/util/index.js
@@ -9,6 +9,7 @@ export { utilHighlightEntities } from './util.js';
 export { utilKeybinding } from './keybinding.js';
 export { utilNoAuto } from './util.js';
 export { utilRebind } from './rebind.js';
+export { utilSanitizeHTML } from './sanitize.js';
 export { utilSetDimensions } from './dimensions.js';
 export { utilSetTransform } from './util.js';
 export { utilTotalExtent } from './util.js';
diff --git a/modules/util/sanitize.js b/modules/util/sanitize.js
@@ -0,0 +1,30 @@
+import DOMPurify from 'dompurify';
+
+/**
+ * Sanitize HTML to prevent XSS attacks.
+ * Uses DOMPurify with a safe default configuration.
+ * This allows safe HTML tags through while stripping dangerous ones.
+ *
+ * @param {string} dirty - The untrusted HTML string to sanitize
+ * @returns {string} The sanitized HTML string
+ */
+export function utilSanitizeHTML(dirty) {
+  if (!dirty) return '';
+
+  return DOMPurify.sanitize(dirty, {
+    ALLOWED_TAGS: [
+      'a', 'b', 'br', 'code', 'em', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6',
+      'hr', 'i', 'img', 'li', 'mark', 'ol', 'option', 'p', 'pre', 'select',
+      'small', 'span', 'strong', 'sub', 'sup', 'table', 'tbody', 'td', 'th',
+      'thead', 'tr', 'u', 'ul'
+    ],
+    ALLOWED_ATTR: [
+      'class', 'href', 'id', 'name', 'rel', 'src', 'target', 'title', 'alt',
+      'value', 'data-osm-id', 'data-osm-type'
+    ],
+    ALLOW_DATA_ATTR: false,
+    ADD_ATTR: ['target'],
+    FORBID_TAGS: ['style', 'script', 'iframe', 'form', 'input', 'button'],
+    FORBID_ATTR: ['onerror', 'onload', 'onclick', 'onmouseover', 'style']
+  });
+}
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -69,6 +69,7 @@
     "@tmcw/togeojson": "^6.0.1",
     "@types/chai": "^5.0.1",
     "d3": "~7.9.0",
+    "dompurify": "^3.3.1",
     "editor-layer-index": "github:osmlab/editor-layer-index#gh-pages",
     "fast-deep-equal": "~3.1.3",
     "fast-json-stable-stringify": "2.1.0",
