From 8e0920d50da82f4b6e605d56f41b69fbb9606a98 Mon Sep 17 00:00:00 2001 From: WofWca Date: Sun, 15 May 2022 12:13:21 +0300 Subject: [PATCH] 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. --- smoothie.js | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/smoothie.js b/smoothie.js index 4380d53..379b4bc 100644 --- a/smoothie.js +++ b/smoothie.js @@ -106,6 +106,7 @@ * 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 @@ -378,20 +379,28 @@ /** 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('' + - label + - this.options.yMaxFormatter(data[i].value, this.options.labels.precision) + ''); + 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('
'); + return elements.innerHTML; }; SmoothieChart.defaultChartOptions = { @@ -627,6 +636,8 @@ } 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 {