From fcbd8a5081a1e335be62c786344ca13abeeb2d10 Mon Sep 17 00:00:00 2001 From: Knut Sveidqvist Date: Wed, 17 Nov 2021 20:08:39 +0100 Subject: [PATCH 1/3] #2496 Fix for issue --- cypress/integration/other/xss.spec.js | 20 +++++ cypress/platform/xss10.html | 105 ++++++++++++++++++++++++++ cypress/platform/xss11.html | 103 +++++++++++++++++++++++++ cypress/platform/xss12.html | 103 +++++++++++++++++++++++++ cypress/platform/xss13.html | 103 +++++++++++++++++++++++++ src/dagre-wrapper/createLabel.js | 6 +- 6 files changed, 437 insertions(+), 3 deletions(-) create mode 100644 cypress/platform/xss10.html create mode 100644 cypress/platform/xss11.html create mode 100644 cypress/platform/xss12.html create mode 100644 cypress/platform/xss13.html diff --git a/cypress/integration/other/xss.spec.js b/cypress/integration/other/xss.spec.js index 9a07a7ab2..cce120ff1 100644 --- a/cypress/integration/other/xss.spec.js +++ b/cypress/integration/other/xss.spec.js @@ -78,5 +78,25 @@ describe('XSS', () => { cy.wait(1000); cy.get('#the-malware').should('not.exist'); }) + it('should not allow maniplulating antiscript to run javascript using onerror in state diagrams with dagre d3', () => { + cy.visit('http://localhost:9000/xss10.html'); + cy.wait(1000); + cy.get('#the-malware').should('not.exist'); + }) + it('should not allow maniplulating antiscript to run javascript using onerror in state diagrams with dagre d3', () => { + cy.visit('http://localhost:9000/xss11.html'); + cy.wait(1000); + cy.get('#the-malware').should('not.exist'); + }) + it('should not allow maniplulating antiscript to run javascript using onerror in state diagrams with dagre d3', () => { + cy.visit('http://localhost:9000/xss12.html'); + cy.wait(1000); + cy.get('#the-malware').should('not.exist'); + }) + it('should not allow maniplulating antiscript to run javascript using onerror in state diagrams with dagre d3', () => { + cy.visit('http://localhost:9000/xss13.html'); + cy.wait(1000); + cy.get('#the-malware').should('not.exist'); + }) }) diff --git a/cypress/platform/xss10.html b/cypress/platform/xss10.html new file mode 100644 index 000000000..3fc10dbab --- /dev/null +++ b/cypress/platform/xss10.html @@ -0,0 +1,105 @@ + + + + + + + + + +
Security check
+
+
+
+ + + + + diff --git a/cypress/platform/xss11.html b/cypress/platform/xss11.html new file mode 100644 index 000000000..8114e055e --- /dev/null +++ b/cypress/platform/xss11.html @@ -0,0 +1,103 @@ + + + + + + + + + +
Security check
+
+
+
+ + + + + diff --git a/cypress/platform/xss12.html b/cypress/platform/xss12.html new file mode 100644 index 000000000..460dd5921 --- /dev/null +++ b/cypress/platform/xss12.html @@ -0,0 +1,103 @@ + + + + + + + + + +
Security check
+
+
+
+ + + + + diff --git a/cypress/platform/xss13.html b/cypress/platform/xss13.html new file mode 100644 index 000000000..48156949e --- /dev/null +++ b/cypress/platform/xss13.html @@ -0,0 +1,103 @@ + + + + + + + + + +
Security check
+
+
+
+ + + + + diff --git a/src/dagre-wrapper/createLabel.js b/src/dagre-wrapper/createLabel.js index bcf8f098e..1137b6bf9 100644 --- a/src/dagre-wrapper/createLabel.js +++ b/src/dagre-wrapper/createLabel.js @@ -1,6 +1,7 @@ import { select } from 'd3'; import { log } from '../logger'; // eslint-disable-line -import { evaluate } from '../diagrams/common/common'; +import { getConfig } from '../config'; +import { evaluate, sanitizeText } from '../diagrams/common/common'; // let vertexNode; // if (evaluate(getConfig().flowchart.htmlLabels)) { // // TODO: addHtmlLabel accepts a labelStyle. Do we possibly have that? @@ -25,7 +26,6 @@ import { evaluate } from '../diagrams/common/common'; // } // vertexNode = svgLabel; // } -import { getConfig } from '../config'; function applyStyle(dom, styleFn) { if (styleFn) { @@ -85,7 +85,7 @@ function addHtmlLabel(node) { } const createLabel = (_vertexText, style, isTitle, isNode) => { - let vertexText = _vertexText || ''; + let vertexText = sanitizeText(_vertexText || '', getConfig()); if (typeof vertexText === 'object') vertexText = vertexText[0]; if (evaluate(getConfig().flowchart.htmlLabels)) { // TODO: addHtmlLabel accepts a labelStyle. Do we possibly have that? From 3feff08d4235347470bea0b51006fb2b47606d2c Mon Sep 17 00:00:00 2001 From: Knut Sveidqvist Date: Wed, 17 Nov 2021 21:31:52 +0100 Subject: [PATCH 2/3] #2496 A more delicate fix for issue --- src/dagre-wrapper/createLabel.js | 2 +- src/diagrams/class/classDb.js | 2 +- src/diagrams/state/stateRenderer-v2.js | 19 +++++++++++-------- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/dagre-wrapper/createLabel.js b/src/dagre-wrapper/createLabel.js index 1137b6bf9..3a2169258 100644 --- a/src/dagre-wrapper/createLabel.js +++ b/src/dagre-wrapper/createLabel.js @@ -85,7 +85,7 @@ function addHtmlLabel(node) { } const createLabel = (_vertexText, style, isTitle, isNode) => { - let vertexText = sanitizeText(_vertexText || '', getConfig()); + let vertexText = _vertexText || ''; if (typeof vertexText === 'object') vertexText = vertexText[0]; if (evaluate(getConfig().flowchart.htmlLabels)) { // TODO: addHtmlLabel accepts a labelStyle. Do we possibly have that? diff --git a/src/diagrams/class/classDb.js b/src/diagrams/class/classDb.js index e4a0cc1c3..027b85564 100644 --- a/src/diagrams/class/classDb.js +++ b/src/diagrams/class/classDb.js @@ -25,7 +25,7 @@ const splitClassNameAndType = function (id) { let split = id.split('~'); className = split[0]; - genericType = split[1]; + genericType = common.sanitizeText(split[1], configApi.getConfig()); } return { className: className, type: genericType }; diff --git a/src/diagrams/state/stateRenderer-v2.js b/src/diagrams/state/stateRenderer-v2.js index 33ee7914d..1f0134896 100644 --- a/src/diagrams/state/stateRenderer-v2.js +++ b/src/diagrams/state/stateRenderer-v2.js @@ -61,20 +61,23 @@ const setupNode = (g, parent, node, altFlag) => { if (Array.isArray(nodeDb[node.id].description)) { // There already is an array of strings,add to it nodeDb[node.id].shape = 'rectWithTitle'; - nodeDb[node.id].description.push(node.description); + nodeDb[node.id].description.push(common.sanitizeText(node.description, getConfig())); } else { if (nodeDb[node.id].description.length > 0) { // if there is a description already transformit to an array nodeDb[node.id].shape = 'rectWithTitle'; if (nodeDb[node.id].description === node.id) { // If the previous description was the is, remove it - nodeDb[node.id].description = [node.description]; + nodeDb[node.id].description = [common.sanitizeText(node.description, getConfig())]; } else { - nodeDb[node.id].description = [nodeDb[node.id].description, node.description]; + nodeDb[node.id].description = [ + common.sanitizeText(nodeDb[node.id].description, getConfig()), + common.sanitizeText(node.description, getConfig()), + ]; } } else { nodeDb[node.id].shape = 'rect'; - nodeDb[node.id].description = node.description; + nodeDb[node.id].description = common.sanitizeText(node.description, getConfig()); } } } @@ -97,7 +100,7 @@ const setupNode = (g, parent, node, altFlag) => { const nodeData = { labelStyle: '', shape: nodeDb[node.id].shape, - labelText: nodeDb[node.id].description, + labelText: common.sanitizeText(nodeDb[node.id].description, getConfig()), // typeof nodeDb[node.id].description === 'object' // ? nodeDb[node.id].description[0] // : nodeDb[node.id].description, @@ -115,7 +118,7 @@ const setupNode = (g, parent, node, altFlag) => { const noteData = { labelStyle: '', shape: 'note', - labelText: node.note.text, + labelText: common.sanitizeText(node.note.text, getConfig()), classes: 'statediagram-note', //classStr, style: '', //styles.style, id: node.id + '----note-' + cnt, @@ -126,7 +129,7 @@ const setupNode = (g, parent, node, altFlag) => { const groupData = { labelStyle: '', shape: 'noteGroup', - labelText: node.note.text, + labelText: common.sanitizeText(node.note.text, getConfig()), classes: nodeDb[node.id].classes, //classStr, style: '', //styles.style, id: node.id + '----parent', @@ -194,7 +197,7 @@ const setupDoc = (g, parent, doc, altFlag) => { arrowTypeEnd: 'arrow_barb', style: 'fill:none', labelStyle: '', - label: item.description, + label: common.sanitizeText(item.description, getConfig()), arrowheadStyle: 'fill: #333', labelpos: 'c', labelType: 'text', From 72d2045104bbc89159f604798bd5c31e0c73b3d4 Mon Sep 17 00:00:00 2001 From: Knut Sveidqvist Date: Wed, 17 Nov 2021 22:30:30 +0100 Subject: [PATCH 3/3] #2496 Unbreaking state diagrams --- src/dagre-wrapper/createLabel.js | 2 +- src/diagrams/common/common.js | 8 ++++++++ src/diagrams/state/stateRenderer-v2.js | 21 +++++++++++---------- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/dagre-wrapper/createLabel.js b/src/dagre-wrapper/createLabel.js index 3a2169258..47ffc911e 100644 --- a/src/dagre-wrapper/createLabel.js +++ b/src/dagre-wrapper/createLabel.js @@ -1,7 +1,7 @@ import { select } from 'd3'; import { log } from '../logger'; // eslint-disable-line import { getConfig } from '../config'; -import { evaluate, sanitizeText } from '../diagrams/common/common'; +import { evaluate } from '../diagrams/common/common'; // let vertexNode; // if (evaluate(getConfig().flowchart.htmlLabels)) { // // TODO: addHtmlLabel accepts a labelStyle. Do we possibly have that? diff --git a/src/diagrams/common/common.js b/src/diagrams/common/common.js index 94769194a..106bb75f9 100644 --- a/src/diagrams/common/common.js +++ b/src/diagrams/common/common.js @@ -80,6 +80,13 @@ export const sanitizeText = (text, config) => { return txt; }; +export const sanitizeTextOrArray = (a, config) => { + if (typeof a === 'string') return sanitizeText(a, config); + + const f = (x) => sanitizeText(x, config); + return a.flat().map(f); +}; + export const lineBreakRegex = //gi; /** @@ -149,6 +156,7 @@ export const evaluate = (val) => (val === 'false' || val === false ? false : tru export default { getRows, sanitizeText, + sanitizeTextOrArray, hasBreaks, splitBreaks, lineBreakRegex, diff --git a/src/diagrams/state/stateRenderer-v2.js b/src/diagrams/state/stateRenderer-v2.js index 1f0134896..417fccb2f 100644 --- a/src/diagrams/state/stateRenderer-v2.js +++ b/src/diagrams/state/stateRenderer-v2.js @@ -61,25 +61,26 @@ const setupNode = (g, parent, node, altFlag) => { if (Array.isArray(nodeDb[node.id].description)) { // There already is an array of strings,add to it nodeDb[node.id].shape = 'rectWithTitle'; - nodeDb[node.id].description.push(common.sanitizeText(node.description, getConfig())); + nodeDb[node.id].description.push(node.description); } else { if (nodeDb[node.id].description.length > 0) { // if there is a description already transformit to an array nodeDb[node.id].shape = 'rectWithTitle'; if (nodeDb[node.id].description === node.id) { // If the previous description was the is, remove it - nodeDb[node.id].description = [common.sanitizeText(node.description, getConfig())]; + nodeDb[node.id].description = [node.description]; } else { - nodeDb[node.id].description = [ - common.sanitizeText(nodeDb[node.id].description, getConfig()), - common.sanitizeText(node.description, getConfig()), - ]; + nodeDb[node.id].description = [nodeDb[node.id].description, node.description]; } } else { nodeDb[node.id].shape = 'rect'; - nodeDb[node.id].description = common.sanitizeText(node.description, getConfig()); + nodeDb[node.id].description = node.description; } } + nodeDb[node.id].description = common.sanitizeTextOrArray( + nodeDb[node.id].description, + getConfig() + ); } // Save data for description and group so that for instance a statement without description overwrites @@ -100,7 +101,7 @@ const setupNode = (g, parent, node, altFlag) => { const nodeData = { labelStyle: '', shape: nodeDb[node.id].shape, - labelText: common.sanitizeText(nodeDb[node.id].description, getConfig()), + labelText: nodeDb[node.id].description, // typeof nodeDb[node.id].description === 'object' // ? nodeDb[node.id].description[0] // : nodeDb[node.id].description, @@ -118,7 +119,7 @@ const setupNode = (g, parent, node, altFlag) => { const noteData = { labelStyle: '', shape: 'note', - labelText: common.sanitizeText(node.note.text, getConfig()), + labelText: node.note.text, classes: 'statediagram-note', //classStr, style: '', //styles.style, id: node.id + '----note-' + cnt, @@ -129,7 +130,7 @@ const setupNode = (g, parent, node, altFlag) => { const groupData = { labelStyle: '', shape: 'noteGroup', - labelText: common.sanitizeText(node.note.text, getConfig()), + labelText: node.note.text, classes: nodeDb[node.id].classes, //classStr, style: '', //styles.style, id: node.id + '----parent',