Merge pull request #1307 from plotly/hover-fix-xss

Fix XSS in trace name on hover

Author: etpinard

src/lib/notifier.js

@@ -63,7 +63,12 @@ module.exports = function(text, displayLength) {
                     note.transition().call(killNote);
                 });
 
-            note.append('p').html(thisText);
+            var p = note.append('p');
+            var lines = thisText.split(/<br\s*\/?>/g);
+            for(var i = 0; i < lines.length; i++) {
+                if(i) p.append('br');
+                p.append('span').text(lines[i]);
+            }
 
             note.transition()
                     .duration(700)

src/lib/svg_text_utils.js

@@ -48,6 +48,7 @@ exports.html_entity_decode = function(s) {
     var replaced = s.replace(/(&[^;]*;)/gi, function(d) {
         if(d === '<') { return '<'; } // special handling for brackets
         if(d === '&rt;') { return '>'; }
+        if(d.indexOf('<') !== -1 || d.indexOf('>') !== -1) { return ''; }
         return hiddenDiv.html(d).text(); // everything else, let the browser decode it to unicode
     });
     hiddenDiv.remove();

src/plots/cartesian/graph_interact.js

@@ -952,12 +952,8 @@ function createHoverText(hoverData, opts) {
 
 
         if(d.name && d.zLabelVal === undefined) {
-            // strip out any html elements from d.name (if it exists at all)
-            // Note that this isn't an XSS vector, only because it never gets
-            // attached to the DOM
-            var tmp = document.createElement('p');
-            tmp.innerHTML = d.name;
-            name = tmp.textContent || '';
+            // strip out our pseudo-html elements from d.name (if it exists at all)
+            name = svgTextUtils.plainText(d.name || '');
 
             if(name.length > 15) name = name.substr(0, 12) + '...';
         }

test/jasmine/tests/hover_label_test.js

@@ -158,6 +158,45 @@ describe('hover info', function() {
         });
     });
 
+    describe('hover info with bad name', function() {
+        var mockCopy = Lib.extendDeep({}, mock);
+
+        mockCopy.data[0].text = [];
+        mockCopy.data[0].text[17] = 'hover text';
+        mockCopy.data[0].hoverinfo = 'all';
+        mockCopy.data[0].name = '<img src=x onerror=y>';
+        mockCopy.data.push({
+            x: [0.002, 0.004],
+            y: [12.5, 16.25],
+            mode: 'lines+markers',
+            name: 'another trace'
+        });
+
+        beforeEach(function(done) {
+            Plotly.plot(createGraphDiv(), mockCopy.data, mockCopy.layout).then(done);
+        });
+
+        it('cleans the name', function() {
+            var gd = document.getElementById('graph');
+            Fx.hover('graph', evt, 'xy');
+
+            var hoverTrace = gd._hoverdata[0];
+
+            expect(hoverTrace.curveNumber).toEqual(0);
+            expect(hoverTrace.pointNumber).toEqual(17);
+            expect(hoverTrace.x).toEqual(0.388);
+            expect(hoverTrace.y).toEqual(1);
+
+            expect(d3.selectAll('g.axistext').size()).toEqual(1);
+            expect(d3.selectAll('g.hovertext').size()).toEqual(1);
+            expect(d3.selectAll('g.axistext').select('text').html()).toEqual('0.388');
+            expect(d3.selectAll('g.hovertext').select('text.nums').selectAll('tspan').size()).toEqual(2);
+            expect(d3.selectAll('g.hovertext').selectAll('tspan')[0][0].innerHTML).toEqual('1');
+            expect(d3.selectAll('g.hovertext').selectAll('tspan')[0][1].innerHTML).toEqual('hover text');
+            expect(d3.selectAll('g.hovertext').selectAll('text.name').node().innerHTML).toEqual('&lt;img src=x o...');
+        });
+    });
+
     describe('hover info y+text', function() {
         var mockCopy = Lib.extendDeep({}, mock);