From 2a9eb7f1238819d92ba3bd336a1916cbabc819b4 Mon Sep 17 00:00:00 2001 From: Harshit Anand Date: Mon, 9 Oct 2023 21:13:53 +0530 Subject: [PATCH 1/8] fix: target blank removed from anchor tag --- packages/mermaid/src/diagrams/common/common.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/mermaid/src/diagrams/common/common.ts b/packages/mermaid/src/diagrams/common/common.ts index e0ca2929dbf..25c6250a9c4 100644 --- a/packages/mermaid/src/diagrams/common/common.ts +++ b/packages/mermaid/src/diagrams/common/common.ts @@ -28,6 +28,21 @@ export const removeScript = (txt: string): string => { return DOMPurify.sanitize(txt); }; +DOMPurify.addHook('afterSanitizeAttributes', function (node) { + // set all elements owning target to target=_blank + if ('target' in node) { + node.setAttribute('target', '_blank'); + node.setAttribute('rel', 'noopener noreferrer'); + } + // set non-HTML/MathML links to xlink:show=new + if ( + !node.hasAttribute('target') && + (node.hasAttribute('xlink:href') || node.hasAttribute('href')) + ) { + node.setAttribute('xlink:show', 'new'); + } +}); + const sanitizeMore = (text: string, config: MermaidConfig) => { if (config.flowchart?.htmlLabels !== false) { const level = config.securityLevel; From c279a9f9ed10ca7f0da62f2287363d69d92aa012 Mon Sep 17 00:00:00 2001 From: Harshit Anand Date: Tue, 10 Oct 2023 01:05:55 +0530 Subject: [PATCH 2/8] fix: clean link unit test resolved --- packages/mermaid/src/diagrams/common/common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mermaid/src/diagrams/common/common.ts b/packages/mermaid/src/diagrams/common/common.ts index 25c6250a9c4..e9d5ca42d37 100644 --- a/packages/mermaid/src/diagrams/common/common.ts +++ b/packages/mermaid/src/diagrams/common/common.ts @@ -30,7 +30,7 @@ export const removeScript = (txt: string): string => { DOMPurify.addHook('afterSanitizeAttributes', function (node) { // set all elements owning target to target=_blank - if ('target' in node) { + if (node.tagName === 'A' && node.hasAttribute('href') && 'target' in node) { node.setAttribute('target', '_blank'); node.setAttribute('rel', 'noopener noreferrer'); } From 345e82abeedb28b56884cc024af808911d7e49de Mon Sep 17 00:00:00 2001 From: Harshit Anand Date: Sat, 14 Oct 2023 00:50:09 +0530 Subject: [PATCH 3/8] fix: removed static target=_blank instaed value will fetched from the target attribute --- .../mermaid/src/diagrams/common/common.ts | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/mermaid/src/diagrams/common/common.ts b/packages/mermaid/src/diagrams/common/common.ts index e9d5ca42d37..84db828435b 100644 --- a/packages/mermaid/src/diagrams/common/common.ts +++ b/packages/mermaid/src/diagrams/common/common.ts @@ -28,18 +28,21 @@ export const removeScript = (txt: string): string => { return DOMPurify.sanitize(txt); }; -DOMPurify.addHook('afterSanitizeAttributes', function (node) { - // set all elements owning target to target=_blank - if (node.tagName === 'A' && node.hasAttribute('href') && 'target' in node) { - node.setAttribute('target', '_blank'); - node.setAttribute('rel', 'noopener noreferrer'); +const TEMPORARY_ATTRIBUTE = 'data-temp-href-target'; + +DOMPurify.addHook('beforeSanitizeAttributes', function (node) { + if (node.tagName === 'A' && node.hasAttribute('target')) { + node.setAttribute(TEMPORARY_ATTRIBUTE, node.getAttribute('target') || ''); } - // set non-HTML/MathML links to xlink:show=new - if ( - !node.hasAttribute('target') && - (node.hasAttribute('xlink:href') || node.hasAttribute('href')) - ) { - node.setAttribute('xlink:show', 'new'); +}); + +DOMPurify.addHook('afterSanitizeAttributes', function (node) { + if (node.tagName === 'A' && node.hasAttribute(TEMPORARY_ATTRIBUTE)) { + node.setAttribute('target', node.getAttribute(TEMPORARY_ATTRIBUTE) || ''); + node.removeAttribute(TEMPORARY_ATTRIBUTE); + if (node.getAttribute('target') === '_blank') { + node.setAttribute('rel', 'noopener'); + } } }); From 111e067df50e8a6d38a25b16448c6656faf98dae Mon Sep 17 00:00:00 2001 From: Harshit Anand Date: Mon, 23 Oct 2023 12:03:57 +0530 Subject: [PATCH 4/8] fix: added type Element to the node used in callback in the addhook function --- .../mermaid/src/diagrams/common/common.ts | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/mermaid/src/diagrams/common/common.ts b/packages/mermaid/src/diagrams/common/common.ts index 84db828435b..28f243845d6 100644 --- a/packages/mermaid/src/diagrams/common/common.ts +++ b/packages/mermaid/src/diagrams/common/common.ts @@ -30,21 +30,21 @@ export const removeScript = (txt: string): string => { const TEMPORARY_ATTRIBUTE = 'data-temp-href-target'; -DOMPurify.addHook('beforeSanitizeAttributes', function (node) { - if (node.tagName === 'A' && node.hasAttribute('target')) { - node.setAttribute(TEMPORARY_ATTRIBUTE, node.getAttribute('target') || ''); - } -}); - -DOMPurify.addHook('afterSanitizeAttributes', function (node) { - if (node.tagName === 'A' && node.hasAttribute(TEMPORARY_ATTRIBUTE)) { - node.setAttribute('target', node.getAttribute(TEMPORARY_ATTRIBUTE) || ''); - node.removeAttribute(TEMPORARY_ATTRIBUTE); - if (node.getAttribute('target') === '_blank') { - node.setAttribute('rel', 'noopener'); - } - } -}); +// DOMPurify.addHook('beforeSanitizeAttributes', (node: Element) => { +// if (node.tagName === 'A' && node.hasAttribute('target')) { +// node.setAttribute(TEMPORARY_ATTRIBUTE, node.getAttribute('target') || ''); +// } +// }); + +// DOMPurify.addHook('afterSanitizeAttributes', (node: Element) => { +// if (node.tagName === 'A' && node.hasAttribute(TEMPORARY_ATTRIBUTE)) { +// node.setAttribute('target', node.getAttribute(TEMPORARY_ATTRIBUTE) || ''); +// node.removeAttribute(TEMPORARY_ATTRIBUTE); +// if (node.getAttribute('target') === '_blank') { +// node.setAttribute('rel', 'noopener'); +// } +// } +// }); const sanitizeMore = (text: string, config: MermaidConfig) => { if (config.flowchart?.htmlLabels !== false) { From 3b8c48dd26bfa16172f3c2c5410bdbaf0b71647f Mon Sep 17 00:00:00 2001 From: Harshit Anand Date: Mon, 23 Oct 2023 12:23:08 +0530 Subject: [PATCH 5/8] fix: added type Element to the node used in callback in the dompurify.addhook --- packages/mermaid/src/diagrams/common/common.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mermaid/src/diagrams/common/common.ts b/packages/mermaid/src/diagrams/common/common.ts index 84db828435b..744c342520e 100644 --- a/packages/mermaid/src/diagrams/common/common.ts +++ b/packages/mermaid/src/diagrams/common/common.ts @@ -30,13 +30,13 @@ export const removeScript = (txt: string): string => { const TEMPORARY_ATTRIBUTE = 'data-temp-href-target'; -DOMPurify.addHook('beforeSanitizeAttributes', function (node) { +DOMPurify.addHook('beforeSanitizeAttributes', (node: Element) => { if (node.tagName === 'A' && node.hasAttribute('target')) { node.setAttribute(TEMPORARY_ATTRIBUTE, node.getAttribute('target') || ''); } }); -DOMPurify.addHook('afterSanitizeAttributes', function (node) { +DOMPurify.addHook('afterSanitizeAttributes', (node: Element) => { if (node.tagName === 'A' && node.hasAttribute(TEMPORARY_ATTRIBUTE)) { node.setAttribute('target', node.getAttribute(TEMPORARY_ATTRIBUTE) || ''); node.removeAttribute(TEMPORARY_ATTRIBUTE); From 7960f94eba2112e3ce54443cce5301991a63f178 Mon Sep 17 00:00:00 2001 From: Harshit Anand Date: Mon, 23 Oct 2023 16:09:51 +0530 Subject: [PATCH 6/8] fix: shifted dompurify.addhook functions inside removescript --- .../mermaid/src/diagrams/common/common.ts | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/packages/mermaid/src/diagrams/common/common.ts b/packages/mermaid/src/diagrams/common/common.ts index 744c342520e..caf43bc6823 100644 --- a/packages/mermaid/src/diagrams/common/common.ts +++ b/packages/mermaid/src/diagrams/common/common.ts @@ -25,26 +25,28 @@ export const getRows = (s?: string): string[] => { * @returns The safer text */ export const removeScript = (txt: string): string => { - return DOMPurify.sanitize(txt); -}; + const TEMPORARY_ATTRIBUTE = 'data-temp-href-target'; -const TEMPORARY_ATTRIBUTE = 'data-temp-href-target'; + DOMPurify.addHook('beforeSanitizeAttributes', (node: Element) => { + if (node.tagName === 'A' && node.hasAttribute('target')) { + node.setAttribute(TEMPORARY_ATTRIBUTE, node.getAttribute('target') || ''); + } + }); -DOMPurify.addHook('beforeSanitizeAttributes', (node: Element) => { - if (node.tagName === 'A' && node.hasAttribute('target')) { - node.setAttribute(TEMPORARY_ATTRIBUTE, node.getAttribute('target') || ''); - } -}); - -DOMPurify.addHook('afterSanitizeAttributes', (node: Element) => { - if (node.tagName === 'A' && node.hasAttribute(TEMPORARY_ATTRIBUTE)) { - node.setAttribute('target', node.getAttribute(TEMPORARY_ATTRIBUTE) || ''); - node.removeAttribute(TEMPORARY_ATTRIBUTE); - if (node.getAttribute('target') === '_blank') { - node.setAttribute('rel', 'noopener'); + const sanitizedText = DOMPurify.sanitize(txt); + + DOMPurify.addHook('afterSanitizeAttributes', (node: Element) => { + if (node.tagName === 'A' && node.hasAttribute(TEMPORARY_ATTRIBUTE)) { + node.setAttribute('target', node.getAttribute(TEMPORARY_ATTRIBUTE) || ''); + node.removeAttribute(TEMPORARY_ATTRIBUTE); + if (node.getAttribute('target') === '_blank') { + node.setAttribute('rel', 'noopener'); + } } - } -}); + }); + + return sanitizedText; +}; const sanitizeMore = (text: string, config: MermaidConfig) => { if (config.flowchart?.htmlLabels !== false) { From 06d2ba8398ec79598a7d6333c1527402c0e7a5de Mon Sep 17 00:00:00 2001 From: Harshit Anand Date: Wed, 25 Oct 2023 21:17:53 +0530 Subject: [PATCH 7/8] fix: added two unit tests to check for the secured anchor tag --- .../mermaid/src/diagrams/common/common.spec.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/mermaid/src/diagrams/common/common.spec.ts b/packages/mermaid/src/diagrams/common/common.spec.ts index 4dac5b33c16..9af24440613 100644 --- a/packages/mermaid/src/diagrams/common/common.spec.ts +++ b/packages/mermaid/src/diagrams/common/common.spec.ts @@ -38,6 +38,20 @@ describe('when securityLevel is antiscript, all script must be removed', () => { compareRemoveScript(``, ``); }); + it('should detect unsecured target attribute, if value is _blank then generate a secured link', () => { + compareRemoveScript( + `note about mermaid`, + `note about mermaid` + ); + }); + + it('should detect unsecured target attribute from links', () => { + compareRemoveScript( + `note about mermaid`, + `note about mermaid` + ); + }); + it('should detect iframes', () => { compareRemoveScript( ` From 54ab3fc3b2dc7f4be13c6f18592ec61662f9c171 Mon Sep 17 00:00:00 2001 From: Harshit Anand Date: Thu, 26 Oct 2023 14:55:04 +0530 Subject: [PATCH 8/8] fix: added an e2e test case for classdiagram with anchor tag --- cypress/integration/rendering/classDiagram.spec.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cypress/integration/rendering/classDiagram.spec.js b/cypress/integration/rendering/classDiagram.spec.js index a23430b0833..cab3649df43 100644 --- a/cypress/integration/rendering/classDiagram.spec.js +++ b/cypress/integration/rendering/classDiagram.spec.js @@ -501,4 +501,16 @@ describe('Class diagram', () => { B : -methods() `); }); + + it('should handle notes with anchor tag having target attribute', () => { + renderGraph( + `classDiagram + class test { } + note for test "note about mermaid"` + ); + + cy.get('svg').then((svg) => { + cy.get('a').should('have.attr', 'target', '_blank').should('have.attr', 'rel', 'noopener'); + }); + }); });