CVE-2022-25929: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: The package smoothie from 1.31.0 and before 1.36.1 are vulnerable to Cross-site Scripting (XSS) due to improper user input sanitization in strokeStyle and tooltipLabel properties. Exploiting this vulnerability is possible when the user can control these properties.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2022-12-21T05:15Z
Repository: https://github.com/joewalnes/smoothie
Language: JavaScript
Files changed: smoothie.js
Commit date: 2022-05-15 12:37:14 +0300
Commit message: fix: potential XSS when `tooltipLabel` or `strokeStyle` are controlled by users

This doesn't completely get rid of `innerHTML` usage, but at least
now the developer is responsible for `tooltipFormatter` not returning
dangerous HTML if they decide to override it.

Diff: diff --git a/smoothie.js b/smoothie.js
index 4380d53..379b4bc 100644
--- a/smoothie.js
+++ b/smoothie.js
@@ -104,10 +104,11 @@
  *        Fix a memory leak appearing when some `timeSeries.disabled === true`, by @WofWca (#132)
  *        Fix: make all lines sharp, remove the `grid.sharpLines` option by @WofWca (#134)
  *        Improve performance, by @WofWca (#135)
  *        Fix `this.delay` not being respected with `nonRealtimeData: true`, by @WofWca (#137)
  *        Fix series fill & stroke being inconsistent for last data time < render time, by @WofWca (#138)
+ * v1.36.1: Fix a potential XSS when `tooltipLabel` or `strokeStyle` are controlled by users, by @WofWca
  */
 
   // Date.now polyfill
   Date.now = Date.now || function() { return new Date().getTime(); };
 
@@ -376,24 +377,32 @@
   }
 
   /** Formats the HTML string content of the tooltip. */
   SmoothieChart.tooltipFormatter = function (timestamp, data) {
       var timestampFormatter = this.options.timestampFormatter || SmoothieChart.timeFormatter,
-          lines = [timestampFormatter(new Date(timestamp))],
+          // A dummy element to hold children. Maybe there's a better way.
+          elements = document.createElement('div'),
           label;
+      elements.appendChild(document.createTextNode(
+        timestampFormatter(new Date(timestamp))
+      ));
 
       for (var i = 0; i < data.length; ++i) {
         label = data[i].series.options.tooltipLabel || ''
         if (label !== ''){
             label = label + ' ';
         }
-        lines.push('<span style="color:' + data[i].series.options.strokeStyle + '">' +
-        label +
-        this.options.yMaxFormatter(data[i].value, this.options.labels.precision) + '</span>');
+        var dataEl = document.createElement('span');
+        dataEl.style.color = data[i].series.options.strokeStyle;
+        dataEl.appendChild(document.createTextNode(
+          label + this.options.yMaxFormatter(data[i].value, this.options.labels.precision)
+        ));
+        elements.appendChild(document.createElement('br'));
+        elements.appendChild(dataEl);
       }
 
-      return lines.join('<br>');
+      return elements.innerHTML;
   };
 
   SmoothieChart.defaultChartOptions = {
     millisPerPixel: 20,
     enableDpiScaling: true,
@@ -625,10 +634,12 @@
         data.push({ series: this.seriesSet[d], index: closeIdx, value: timeSeries.data[closeIdx][1] });
       }
     }
 
     if (data.length) {
+      // TODO make `tooltipFormatter` return element(s) instead of an HTML string so it's harder for users
+      // to introduce an XSS. This would be a breaking change.
       el.innerHTML = this.options.tooltipFormatter.call(this, t, data);
       el.style.display = 'block';
     } else {
       el.style.display = 'none';
     }

Vulnerable code:           lines = [timestampFormatter(new Date(timestamp))],
        lines.push('<span style="color:' + data[i].series.options.strokeStyle + '">' +
        label +
        this.options.yMaxFormatter(data[i].value, this.options.labels.precision) + '</span>');
      return lines.join('<br>');
Fixed code:  * v1.36.1: Fix a potential XSS when `tooltipLabel` or `strokeStyle` are controlled by users, by @WofWca
          // A dummy element to hold children. Maybe there's a better way.
          elements = document.createElement('div'),
      elements.appendChild(document.createTextNode(
        timestampFormatter(new Date(timestamp))
      ));
        var dataEl = document.createElement('span');
        dataEl.style.color = data[i].series.options.strokeStyle;
        dataEl.appendChild(document.createTextNode(
          label + this.options.yMaxFormatter(data[i].value, this.options.labels.precision)
        ));
        elements.appendChild(document.createElement('br'));
        elements.appendChild(dataEl);
      return elements.innerHTML;
      // TODO make `tooltipFormatter` return element(s) instead of an HTML string so it's harder for users
      // to introduce an XSS. This would be a breaking change.