CVE-2018-20801: Incorrect Regular Expression (CWE-185)
value: In js/parts/SvgRenderer.js in Highcharts JS before 6.1.0, the use of backtracking regular expressions permitted an attacker to conduct a denial of service attack against the SVGRenderer component, aka ReDoS.
CWE description: The product specifies a regular expression in a way that causes data to be improperly matched or compared.
CVSS v3 base score: 7.5
Published: 2019-03-14T16:29Z
Repository: https://github.com/highcharts/highcharts
Language: JavaScript
Files changed: js/parts/SvgRenderer.js, package.json, samples/unit-tests/svgrenderer/text/demo.js
Commit date: 2018-03-01 10:37:21 +0100
Commit message: Refactored SVGRenderer text to not use backtracking regexes to replace attributes.

Adds support for single quoted class, href and style attributes in strings.

Diff: diff --git a/js/parts/SvgRenderer.js b/js/parts/SvgRenderer.js
index fb3672b65..545392bac 100644
--- a/js/parts/SvgRenderer.js
+++ b/js/parts/SvgRenderer.js
@@ -2346,13 +2346,10 @@ extend(SVGRenderer.prototype, /** @lends Highcharts.SVGRenderer.prototype */ {
 			forExport = renderer.forExport,
 			textStr = pick(wrapper.textStr, '').toString(),
 			hasMarkup = textStr.indexOf('<') !== -1,
 			lines,
 			childNodes = textNode.childNodes,
-			clsRegex,
-			styleRegex,
-			hrefRegex,
 			wasTooLong,
 			parentX = attr(textNode, 'x'),
 			textStyles = wrapper.styles,
 			width = wrapper.textWidth,
 			textLineHeight = textStyles && textStyles.lineHeight,
@@ -2388,10 +2385,27 @@ extend(SVGRenderer.prototype, /** @lends Highcharts.SVGRenderer.prototype */ {
 							key
 						);
 					}
 				});
 				return inputStr;
+			},
+			parseAttribute = function (s, attr) {
+				var start,
+					delimiter;
+
+				start = s.indexOf('<');
+				s = s.substring(start, s.indexOf('>') - start);
+
+				start = s.indexOf(attr + '=');
+				if (start !== -1) {
+					start = start + attr.length + 1;
+					delimiter = s.charAt(start);
+					if (delimiter === '"' || delimiter === "'") { // eslint-disable-line quotes
+						s = s.substring(start + 1);
+						return s.substring(0, s.indexOf(delimiter));
+					}
+				}
 			};
 
 		// The buildText code is quite heavy, so if we're not changing something
 		// that affects the text, skip it (#6113).
 		textCache = [
@@ -2425,14 +2439,10 @@ extend(SVGRenderer.prototype, /** @lends Highcharts.SVGRenderer.prototype */ {
 			textNode.appendChild(doc.createTextNode(unescapeEntities(textStr)));
 
 		// Complex strings, add more logic
 		} else {
 
-			clsRegex = /<.*class="([^"]+)".*>/;
-			styleRegex = /<.*style="([^"]+)".*>/;
-			hrefRegex = /<.*href="([^"]+)".*>/;
-
 			if (tempParent) {
 				// attach it to the DOM to read offset width
 				tempParent.appendChild(textNode);
 			}
 
@@ -2483,31 +2493,35 @@ extend(SVGRenderer.prototype, /** @lends Highcharts.SVGRenderer.prototype */ {
 						var attributes = {},
 							tspan = doc.createElementNS(
 								renderer.SVG_NS,
 								'tspan'
 							),
-							spanCls,
-							spanStyle; // #390
-						if (clsRegex.test(span)) {
-							spanCls = span.match(clsRegex)[1];
-							attr(tspan, 'class', spanCls);
+							classAttribute,
+							styleAttribute, // #390
+							hrefAttribute;
+						
+						classAttribute = parseAttribute(span, 'class');
+						if (classAttribute) {
+							attr(tspan, 'class', classAttribute);
 						}
-						if (styleRegex.test(span)) {
-							spanStyle = span.match(styleRegex)[1].replace(
+
+						styleAttribute = parseAttribute(span, 'style');
+						if (styleAttribute) {
+							styleAttribute = styleAttribute.replace(
 								/(;| |^)color([ :])/,
 								'$1fill$2'
 							);
-							attr(tspan, 'style', spanStyle);
+							attr(tspan, 'style', styleAttribute);
 						}
 
 						// Not for export - #1529
-						if (hrefRegex.test(span) && !forExport) {
+						hrefAttribute = parseAttribute(span, 'href');
+						if (hrefAttribute && !forExport) {
 							attr(
 								tspan,
 								'onclick',
-								'location.href=\"' +
-									span.match(hrefRegex)[1] + '\"'
+								'location.href=\"' + hrefAttribute + '\"'
 							);
 							attr(tspan, 'class', 'highcharts-anchor');
 							/*= if (build.classic) { =*/
 							css(tspan, { cursor: 'pointer' });
 							/*= } =*/
@@ -2647,12 +2661,16 @@ extend(SVGRenderer.prototype, /** @lends Highcharts.SVGRenderer.prototype */ {
 											);
 											attr(tspan, {
 												dy: dy,
 												x: parentX
 											});
-											if (spanStyle) { // #390
-												attr(tspan, 'style', spanStyle);
+											if (styleAttribute) { // #390
+												attr(
+													tspan,
+													'style',
+													styleAttribute
+												);
 											}
 											textNode.appendChild(tspan);
 										}
 
 										// a single word is pressing it out
diff --git a/package.json b/package.json
index 6069c9214..b0d34c7e4 100644
--- a/package.json
+++ b/package.json
@@ -80,8 +80,9 @@
   "license": "SEE LICENSE IN <license.txt>",
   "dependencies": {
     "aws-sdk": "^2.94.0",
     "babel-runtime": "^6.20.0",
     "glob": "^7.1.2",
+    "safe-regex": "^1.1.0",
     "taffydb": "^2.7.3"
   }
 }
diff --git a/samples/unit-tests/svgrenderer/text/demo.js b/samples/unit-tests/svgrenderer/text/demo.js
index 4dc68aa48..dca8b4316 100644
--- a/samples/unit-tests/svgrenderer/text/demo.js
+++ b/samples/unit-tests/svgrenderer/text/demo.js
@@ -202,5 +202,41 @@ QUnit.test('Dir rtl (#3482)', function (assert) {
         'Label sits nicely inside box'
     );
 
     document.getElementById('container').removeAttribute('dir');
 });
+
+QUnit.test('Attributes', function (assert) {
+    var ren = new Highcharts.Renderer(
+        document.getElementById('container'),
+        600,
+        400
+    );
+
+    var text = ren
+        .text(
+            'The quick brown fox jumps <span class="red">over</span> the lazy dog',
+            20,
+            20
+        )
+        .add();
+
+    assert.strictEqual(
+        text.element.childNodes[1].getAttribute('class'),
+        'red',
+        'Double quotes, red span should be picked up'
+    );
+
+    text = ren
+        .text(
+            "The quick brown fox jumps <span class='red'>over</span> the lazy dog",
+            20,
+            20
+        )
+        .add();
+
+    assert.strictEqual(
+        text.element.childNodes[1].getAttribute('class'),
+        'red',
+        'Single quotes, red span should be picked up'
+    );
+});

Vulnerable code: 			clsRegex,
			styleRegex,
			hrefRegex,
			clsRegex = /<.*class="([^"]+)".*>/;
			styleRegex = /<.*style="([^"]+)".*>/;
			hrefRegex = /<.*href="([^"]+)".*>/;

							spanCls,
							spanStyle; // #390
						if (clsRegex.test(span)) {
							spanCls = span.match(clsRegex)[1];
							attr(tspan, 'class', spanCls);
						if (styleRegex.test(span)) {
							spanStyle = span.match(styleRegex)[1].replace(
							attr(tspan, 'style', spanStyle);
						if (hrefRegex.test(span) && !forExport) {
								'location.href=\"' +
									span.match(hrefRegex)[1] + '\"'
											if (spanStyle) { // #390
												attr(tspan, 'style', spanStyle);
Fixed code: 			},
			parseAttribute = function (s, attr) {
				var start,
					delimiter;

				start = s.indexOf('<');
				s = s.substring(start, s.indexOf('>') - start);

				start = s.indexOf(attr + '=');
				if (start !== -1) {
					start = start + attr.length + 1;
					delimiter = s.charAt(start);
					if (delimiter === '"' || delimiter === "'") { // eslint-disable-line quotes
						s = s.substring(start + 1);
						return s.substring(0, s.indexOf(delimiter));
					}
				}
							classAttribute,
							styleAttribute, // #390
							hrefAttribute;
						
						classAttribute = parseAttribute(span, 'class');
						if (classAttribute) {
							attr(tspan, 'class', classAttribute);

						styleAttribute = parseAttribute(span, 'style');
						if (styleAttribute) {
							styleAttribute = styleAttribute.replace(
							attr(tspan, 'style', styleAttribute);
						hrefAttribute = parseAttribute(span, 'href');
						if (hrefAttribute && !forExport) {
								'location.href=\"' + hrefAttribute + '\"'
											if (styleAttribute) { // #390
												attr(
													tspan,
													'style',
													styleAttribute
												);
    "safe-regex": "^1.1.0",

QUnit.test('Attributes', function (assert) {
    var ren = new Highcharts.Renderer(
        document.getElementById('container'),
        600,
        400
    );

    var text = ren
        .text(
            'The quick brown fox jumps <span class="red">over</span> the lazy dog',
            20,
            20
        )
        .add();

    assert.strictEqual(
        text.element.childNodes[1].getAttribute('class'),
        'red',
        'Double quotes, red span should be picked up'
    );

    text = ren
        .text(
            "The quick brown fox jumps <span class='red'>over</span> the lazy dog",
            20,
            20
        )
        .add();

    assert.strictEqual(
        text.element.childNodes[1].getAttribute('class'),
        'red',
        'Single quotes, red span should be picked up'
    );
});