Fixed #13559, JS execution allowed from chart config

highcharts · May 20, 2020 · 55c39dd · 55c39dd
1 parent d8237ea
commit 55c39dd
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 18 deletions.
diff --git a/js/parts/SVGRenderer.js b/js/parts/SVGRenderer.js
@@ -813,7 +813,7 @@ var SVGRenderer = /** @class */ (function () {
                 spans = line.split('|||');
                 spans.forEach(function buildTextSpans(span) {
                     if (span !== '' || spans.length === 1) {
-                        var attributes = {}, tspan = doc.createElementNS(renderer.SVG_NS, 'tspan'), classAttribute, styleAttribute, // #390
+                        var attributes = {}, tspan = doc.createElementNS(renderer.SVG_NS, 'tspan'), a, classAttribute, styleAttribute, // #390
                         hrefAttribute;
                         classAttribute = parseAttribute(span, 'class');
                         if (classAttribute) {
@@ -824,13 +824,22 @@ var SVGRenderer = /** @class */ (function () {
                             styleAttribute = styleAttribute.replace(/(;| |^)color([ :])/, '$1fill$2');
                             attr(tspan, 'style', styleAttribute);
                         }
-                        // Not for export - #1529
+                        // For anchors, wrap the tspan in an <a> tag and apply
+                        // the href attribute as is (#13559). Not for export
+                        // (#1529)
                         hrefAttribute = parseAttribute(span, 'href');
                         if (hrefAttribute && !forExport) {
-                            attr(tspan, 'onclick', 'location.href=\"' + hrefAttribute + '\"');
-                            attr(tspan, 'class', 'highcharts-anchor');
-                            if (!renderer.styledMode) {
-                                css(tspan, { cursor: 'pointer' });
+                            if (
+                            // Stop JavaScript links, vulnerable to XSS
+                            hrefAttribute.split(':')[0].toLowerCase()
+                                .indexOf('javascript') === -1) {
+                                a = doc.createElementNS(renderer.SVG_NS, 'a');
+                                attr(a, 'href', hrefAttribute);
+                                attr(tspan, 'class', 'highcharts-anchor');
+                                a.appendChild(tspan);
+                                if (!renderer.styledMode) {
+                                    css(tspan, { cursor: 'pointer' });
+                                }
                             }
                         }
                         // Strip away unsupported HTML tags (#7126)
@@ -852,7 +861,7 @@ var SVGRenderer = /** @class */ (function () {
                             // add attributes
                             attr(tspan, attributes);
                             // Append it
-                            textNode.appendChild(tspan);
+                            textNode.appendChild(a || tspan);
                             // first span on subsequent line, add the line
                             // height
                             if (!spanNo && isSubsequentLine) {

diff --git a/samples/unit-tests/svgrenderer/text/demo.js b/samples/unit-tests/svgrenderer/text/demo.js
@@ -801,3 +801,37 @@ QUnit.test('RTL characters with outline (#10162)', function (assert) {
         renderer.destroy();
     }
 });
+
+QUnit.test('XSS and script injection', assert => {
+    const ren = new Highcharts.Renderer(
+        document.getElementById('container'),
+        600,
+        400
+    );
+
+    ren.text(
+        'This is a link to <a href="https://www.highcharts.com">highcharts.com</a>',
+        30,
+        30
+    )
+        .add();
+
+    assert.strictEqual(
+        document.getElementById('container').innerHTML.indexOf('onclick'),
+        -1,
+        'There should be no translation of anchors to onclick like historically'
+    );
+
+    ren.text(
+        'This is a link to <a href="javascript:alert(\'XSS\')">an alert</a>',
+        30,
+        60
+    )
+        .add();
+
+    assert.strictEqual(
+        document.getElementById('container').innerHTML.indexOf('javascript'),
+        -1,
+        'JavaScript execution should not be allowed from config'
+    );
+});
diff --git a/ts/parts/SVGRenderer.ts b/ts/parts/SVGRenderer.ts
@@ -1300,6 +1300,7 @@ class SVGRenderer {
                                 renderer.SVG_NS,
                                 'tspan'
                             ) as any,
+                            a,
                             classAttribute,
                             styleAttribute, // #390
                             hrefAttribute;
@@ -1318,18 +1319,27 @@ class SVGRenderer {
                             attr(tspan, 'style', styleAttribute);
                         }
 
-                        // Not for export - #1529
+                        // For anchors, wrap the tspan in an <a> tag and apply
+                        // the href attribute as is (#13559). Not for export
+                        // (#1529)
                         hrefAttribute = parseAttribute(span, 'href');
                         if (hrefAttribute && !forExport) {
-                            attr(
-                                tspan,
-                                'onclick',
-                                'location.href=\"' + hrefAttribute + '\"'
-                            );
-                            attr(tspan, 'class', 'highcharts-anchor');
-
-                            if (!renderer.styledMode) {
-                                css(tspan, { cursor: 'pointer' });
+                            if (
+                                // Stop JavaScript links, vulnerable to XSS
+                                hrefAttribute.split(':')[0].toLowerCase()
+                                    .indexOf('javascript') === -1
+                            ) {
+                                a = doc.createElementNS(
+                                    renderer.SVG_NS,
+                                    'a'
+                                ) as any;
+                                attr(a, 'href', hrefAttribute);
+                                attr(tspan, 'class', 'highcharts-anchor');
+                                a.appendChild(tspan);
+
+                                if (!renderer.styledMode) {
+                                    css(tspan, { cursor: 'pointer' });
+                                }
                             }
                         }
 
@@ -1358,7 +1368,7 @@ class SVGRenderer {
                             attr(tspan, attributes);
 
                             // Append it
-                            textNode.appendChild(tspan);
+                            textNode.appendChild(a || tspan);
 
                             // first span on subsequent line, add the line
                             // height