From ec2da8e85d56e2ade16eb687a857ccbcde09d592 Mon Sep 17 00:00:00 2001 From: Knut Sveidqvist Date: Tue, 21 Jun 2022 21:17:53 +0200 Subject: [PATCH 1/4] Only allowing a subset of characters in themeVariables --- cypress/e2e/other/ghsa.spec.js | 10 +++++++ cypress/helpers/util.js | 50 ++++++++++++++++++++++++++++++++++ cypress/platform/ghsa1.html | 28 +++++++++++++++++++ src/styles.js | 6 +++- src/utils.js | 11 ++++++++ 5 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 cypress/e2e/other/ghsa.spec.js create mode 100644 cypress/platform/ghsa1.html diff --git a/cypress/e2e/other/ghsa.spec.js b/cypress/e2e/other/ghsa.spec.js new file mode 100644 index 0000000000..5b168a8a8f --- /dev/null +++ b/cypress/e2e/other/ghsa.spec.js @@ -0,0 +1,10 @@ +import { urlSnapshotTest } from '../../helpers/util'; + +describe('CSS injections', () => { + it('should not allow CSS injections outside of the diagram', () => { + urlSnapshotTest('http://localhost:9000/ghsa1.html', { + logLevel: 1, + flowchart: { htmlLabels: false }, + }); + }); +}); diff --git a/cypress/helpers/util.js b/cypress/helpers/util.js index bef4099365..dd3fdd2c92 100644 --- a/cypress/helpers/util.js +++ b/cypress/helpers/util.js @@ -70,6 +70,56 @@ export const imgSnapshotTest = (graphStr, _options, api = false, validation) => } }; +export const urlSnapshotTest = (url, _options, api = false, validation) => { + cy.log(_options); + const options = Object.assign(_options); + if (!options.fontFamily) { + options.fontFamily = 'courier'; + } + if (!options.sequence) { + options.sequence = {}; + } + if (!options.sequence || (options.sequence && !options.sequence.actorFontFamily)) { + options.sequence.actorFontFamily = 'courier'; + } + if (options.sequence && !options.sequence.noteFontFamily) { + options.sequence.noteFontFamily = 'courier'; + } + options.sequence.actorFontFamily = 'courier'; + options.sequence.noteFontFamily = 'courier'; + options.sequence.messageFontFamily = 'courier'; + if (options.sequence && !options.sequence.actorFontFamily) { + options.sequence.actorFontFamily = 'courier'; + } + if (!options.fontSize) { + options.fontSize = '16px'; + } + const useAppli = Cypress.env('useAppli'); + const branch = Cypress.env('codeBranch'); + cy.log('Hello ' + useAppli ? 'Appli' : 'image-snapshot'); + const name = (options.name || cy.state('runnable').fullTitle()).replace(/\s+/g, '-'); + + if (useAppli) { + cy.eyesOpen({ + appName: 'Mermaid-' + branch, + testName: name, + batchName: branch, + }); + } + + cy.visit(url); + if (validation) cy.get('svg').should(validation); + cy.get('body'); + // Default name to test title + + if (useAppli) { + cy.eyesCheckWindow('Click!'); + cy.eyesClose(); + } else { + cy.matchImageSnapshot(name); + } +}; + export const renderGraph = (graphStr, options, api) => { const url = mermaidUrl(graphStr, options, api); diff --git a/cypress/platform/ghsa1.html b/cypress/platform/ghsa1.html new file mode 100644 index 0000000000..bf2008d7ed --- /dev/null +++ b/cypress/platform/ghsa1.html @@ -0,0 +1,28 @@ + + + +
+

This element does not belong to the SVG but we can style it

+
+ + + + + + + + diff --git a/src/styles.js b/src/styles.js index d965af6314..15e804ef98 100644 --- a/src/styles.js +++ b/src/styles.js @@ -10,6 +10,7 @@ import sequence from './diagrams/sequence/styles'; import stateDiagram from './diagrams/state/styles'; import journey from './diagrams/user-journey/styles'; import c4 from './diagrams/c4/styles'; +import { log } from './logger'; const themes = { flowchart, @@ -30,7 +31,10 @@ const themes = { c4, }; -export const calcThemeVariables = (theme, userOverRides) => theme.calcColors(userOverRides); +export const calcThemeVariables = (theme, userOverRides) => { + log.info('userOverides', userOverRides); + return theme.calcColors(userOverRides); +}; const getStyles = (type, userStyles, options) => { return ` { diff --git a/src/utils.js b/src/utils.js index 7d44e7f7a2..9b8387cc48 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1044,6 +1044,17 @@ export const directiveSanitizer = (args) => { }); } } + if (args.themeVariables) { + const kArr = Object.keys(args.themeVariables); + for (let i = 0; i < kArr.length; i++) { + const k = kArr[i]; + const val = args.themeVariables[k]; + if (!val.match(/^[a-zA-Z0-9#;]+$/)) { + args.themeVariables[k] = ''; + } + } + } + log.debug('After sanitization', args); }; export const sanitizeCss = (str) => { const stringsearch = 'o'; From 610f154c740e04edb8aa5d837b51654d0d898d6f Mon Sep 17 00:00:00 2001 From: Knut Sveidqvist Date: Tue, 21 Jun 2022 21:46:37 +0200 Subject: [PATCH 2/4] Sanitizsation of incoming variables that are added to the userStyles --- cypress/platform/ghsa2.html | 28 ++++++++++++++++++++++++++++ src/mermaidAPI.js | 2 ++ src/utils.js | 8 ++++++++ 3 files changed, 38 insertions(+) create mode 100644 cypress/platform/ghsa2.html diff --git a/cypress/platform/ghsa2.html b/cypress/platform/ghsa2.html new file mode 100644 index 0000000000..b4e390c6c4 --- /dev/null +++ b/cypress/platform/ghsa2.html @@ -0,0 +1,28 @@ + + + +
+

This element does not belong to the SVG but we can style it

+
+ + + + + + + + diff --git a/src/mermaidAPI.js b/src/mermaidAPI.js index efa93791a8..750c73cb1a 100644 --- a/src/mermaidAPI.js +++ b/src/mermaidAPI.js @@ -385,6 +385,8 @@ const render = function (id, _txt, cb, container) { let userStyles = ''; // user provided theme CSS + // If you add more configuration driven data into the user styles make sure that the value is + // sanitized bye the santiizeCSS function if (cnf.themeCSS !== undefined) { userStyles += `\n${cnf.themeCSS}`; } diff --git a/src/utils.js b/src/utils.js index 9b8387cc48..4d7854c226 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1032,6 +1032,14 @@ export const directiveSanitizer = (args) => { log.debug('sanitizing themeCss option'); args[key] = sanitizeCss(args[key]); } + if (key.indexOf('fontFamily') >= 0) { + log.debug('sanitizing fontFamily option'); + args[key] = sanitizeCss(args[key]); + } + if (key.indexOf('altFontFamily') >= 0) { + log.debug('sanitizing altFontFamily option'); + args[key] = sanitizeCss(args[key]); + } if (configKeys.indexOf(key) < 0) { log.debug('sanitize deleting option', key); delete args[key]; From 2792bb41de047e2b7587d4ac5ce050a7eebc4f7c Mon Sep 17 00:00:00 2001 From: Knut Sveidqvist Date: Wed, 22 Jun 2022 08:16:55 +0200 Subject: [PATCH 3/4] Updated allowed characters to accomodate fonts and rgba colors --- src/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.js b/src/utils.js index 4d7854c226..805556dd6a 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1057,7 +1057,7 @@ export const directiveSanitizer = (args) => { for (let i = 0; i < kArr.length; i++) { const k = kArr[i]; const val = args.themeVariables[k]; - if (!val.match(/^[a-zA-Z0-9#;]+$/)) { + if (!val.match(/^[a-zA-Z0-9#,";()%. ]+$/)) { args.themeVariables[k] = ''; } } From 5110e427869b4c5a660d69968909ca7e98e4af2b Mon Sep 17 00:00:00 2001 From: Knut Sveidqvist Date: Tue, 28 Jun 2022 18:50:41 +0200 Subject: [PATCH 4/4] Better balance check --- src/utils.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/utils.js b/src/utils.js index 805556dd6a..be28ef8df7 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1057,7 +1057,7 @@ export const directiveSanitizer = (args) => { for (let i = 0; i < kArr.length; i++) { const k = kArr[i]; const val = args.themeVariables[k]; - if (!val.match(/^[a-zA-Z0-9#,";()%. ]+$/)) { + if (val && val.match && !val.match(/^[a-zA-Z0-9#,";()%. ]+$/)) { args.themeVariables[k] = ''; } } @@ -1065,9 +1065,19 @@ export const directiveSanitizer = (args) => { log.debug('After sanitization', args); }; export const sanitizeCss = (str) => { - const stringsearch = 'o'; - const startCnt = (str.match(/\{/g) || []).length; - const endCnt = (str.match(/\}/g) || []).length; + let startCnt = 0; + let endCnt = 0; + + for (let i = 0; i < str.length; i++) { + if (startCnt < endCnt) { + return '{ /* ERROR: Unbalanced CSS */ }'; + } + if (str[i] === '{') { + startCnt++; + } else if (str[i] === '}') { + endCnt++; + } + } if (startCnt !== endCnt) { return '{ /* ERROR: Unbalanced CSS */ }'; }