From 3afb49ef1076cce5083689491c6c8ca9262bda1e Mon Sep 17 00:00:00 2001 From: MUDRY Julien Date: Wed, 15 May 2024 09:05:56 +0200 Subject: [PATCH 1/3] fix(extension-link): make the javascript link detection case insensitive --- packages/extension-link/src/link.ts | 2 +- .../integration/extensions/link.spec.ts | 82 ++++++++++--------- .../integration/extensions/youtube.spec.ts | 2 + 3 files changed, 48 insertions(+), 38 deletions(-) diff --git a/packages/extension-link/src/link.ts b/packages/extension-link/src/link.ts index e3af421de65..eab40e7bfbf 100644 --- a/packages/extension-link/src/link.ts +++ b/packages/extension-link/src/link.ts @@ -163,7 +163,7 @@ export const Link = Mark.create({ renderHTML({ HTMLAttributes }) { // False positive; we're explicitly checking for javascript: links to ignore them // eslint-disable-next-line no-script-url - if (HTMLAttributes.href?.startsWith('javascript:')) { + if (HTMLAttributes.href?.toLowerCase().startsWith('javascript:')) { // strip out the href return ['a', mergeAttributes(this.options.HTMLAttributes, { ...HTMLAttributes, href: '' }), 0] } diff --git a/tests/cypress/integration/extensions/link.spec.ts b/tests/cypress/integration/extensions/link.spec.ts index edb729093d5..12ffb5a461f 100644 --- a/tests/cypress/integration/extensions/link.spec.ts +++ b/tests/cypress/integration/extensions/link.spec.ts @@ -20,45 +20,53 @@ describe('extension-link', () => { } const getEditorEl = () => document.querySelector(`.${editorElClass}`) - it('does not output src tag for javascript schema', () => { - editor = new Editor({ - element: createEditorEl(), - extensions: [ - Document, - Text, - Paragraph, - Link, - ], - content: { - type: 'doc', - content: [ - { - type: 'paragraph', - content: [ - { - type: 'text', - text: 'hello world!', - marks: [ - { - type: 'link', - attrs: { - // We have to disable the eslint rule here because we're trying to purposely test eval urls - // eslint-disable-next-line no-script-url - href: 'javascript:alert(window.origin)', - }, - }, - ], - }, - ], - }, + const invalidUrls = [ + // We have to disable the eslint rule here because we're trying to purposely test eval urls + // eslint-disable-next-line no-script-url + 'javascript:alert(window.origin)', + // eslint-disable-next-line no-script-url + 'jAvAsCrIpT:alert(window.origin)', + ] + + invalidUrls.forEach(url => { + it('does not output src tag for javascript schema', () => { + editor = new Editor({ + element: createEditorEl(), + extensions: [ + Document, + Text, + Paragraph, + Link, ], - }, - }) + content: { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { + type: 'text', + text: 'hello world!', + marks: [ + { + type: 'link', + attrs: { + href: url, + }, + }, + ], + }, + ], + }, + ], + }, + }) - // eslint-disable-next-line no-script-url - expect(editor.getHTML()).to.not.include('javascript:alert(window.origin)') + // eslint-disable-next-line no-script-url + expect(editor.getHTML()).to.not.include('javascript:alert(window.origin)') - editor?.destroy() - getEditorEl()?.remove() + editor?.destroy() + getEditorEl()?.remove() + }) }) }) diff --git a/tests/cypress/integration/extensions/youtube.spec.ts b/tests/cypress/integration/extensions/youtube.spec.ts index 698d2816ab4..bd542959c5c 100644 --- a/tests/cypress/integration/extensions/youtube.spec.ts +++ b/tests/cypress/integration/extensions/youtube.spec.ts @@ -24,6 +24,8 @@ describe('extension-youtube', () => { // We have to disable the eslint rule here because we're trying to purposely test eval urls // eslint-disable-next-line no-script-url 'javascript:alert(window.origin)//embed/', + // eslint-disable-next-line no-script-url + 'jAvAsCrIpT:alert(window.origin)//embed/', 'https://youtube.google.com/embed/fdsafsdf', 'https://youtube.com.bad/embed', ] From b79de07929ba2be874137b5b107a2d9f6dd03e64 Mon Sep 17 00:00:00 2001 From: MUDRY Julien Date: Wed, 15 May 2024 16:37:55 +0200 Subject: [PATCH 2/3] fix(extension-link): trim the URL before checking the scheme --- packages/extension-link/src/link.ts | 2 +- tests/cypress/integration/extensions/link.spec.ts | 2 ++ tests/cypress/integration/extensions/youtube.spec.ts | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/extension-link/src/link.ts b/packages/extension-link/src/link.ts index eab40e7bfbf..39f5368c59a 100644 --- a/packages/extension-link/src/link.ts +++ b/packages/extension-link/src/link.ts @@ -163,7 +163,7 @@ export const Link = Mark.create({ renderHTML({ HTMLAttributes }) { // False positive; we're explicitly checking for javascript: links to ignore them // eslint-disable-next-line no-script-url - if (HTMLAttributes.href?.toLowerCase().startsWith('javascript:')) { + if (HTMLAttributes.href?.toLowerCase().trim().startsWith('javascript:')) { // strip out the href return ['a', mergeAttributes(this.options.HTMLAttributes, { ...HTMLAttributes, href: '' }), 0] } diff --git a/tests/cypress/integration/extensions/link.spec.ts b/tests/cypress/integration/extensions/link.spec.ts index 12ffb5a461f..9495264607c 100644 --- a/tests/cypress/integration/extensions/link.spec.ts +++ b/tests/cypress/integration/extensions/link.spec.ts @@ -25,6 +25,8 @@ describe('extension-link', () => { // eslint-disable-next-line no-script-url 'javascript:alert(window.origin)', // eslint-disable-next-line no-script-url + ' javascript:alert(window.origin)', + // eslint-disable-next-line no-script-url 'jAvAsCrIpT:alert(window.origin)', ] diff --git a/tests/cypress/integration/extensions/youtube.spec.ts b/tests/cypress/integration/extensions/youtube.spec.ts index bd542959c5c..2d9f5fc2024 100644 --- a/tests/cypress/integration/extensions/youtube.spec.ts +++ b/tests/cypress/integration/extensions/youtube.spec.ts @@ -25,6 +25,8 @@ describe('extension-youtube', () => { // eslint-disable-next-line no-script-url 'javascript:alert(window.origin)//embed/', // eslint-disable-next-line no-script-url + ' javascript:alert(window.origin)', + // eslint-disable-next-line no-script-url 'jAvAsCrIpT:alert(window.origin)//embed/', 'https://youtube.google.com/embed/fdsafsdf', 'https://youtube.com.bad/embed', From 0ac4249143a78066a74f12f9834407d396019640 Mon Sep 17 00:00:00 2001 From: MUDRY Julien Date: Thu, 16 May 2024 07:40:49 +0200 Subject: [PATCH 3/3] fix(extension-link): extract the scheme from the URL before checking whether it's javascript --- packages/extension-link/src/link.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/extension-link/src/link.ts b/packages/extension-link/src/link.ts index 39f5368c59a..2926ea43dbd 100644 --- a/packages/extension-link/src/link.ts +++ b/packages/extension-link/src/link.ts @@ -161,9 +161,7 @@ export const Link = Mark.create({ }, renderHTML({ HTMLAttributes }) { - // False positive; we're explicitly checking for javascript: links to ignore them - // eslint-disable-next-line no-script-url - if (HTMLAttributes.href?.toLowerCase().trim().startsWith('javascript:')) { + if (HTMLAttributes.href?.substring(0, HTMLAttributes.href.indexOf(':')).toLowerCase().trim() === 'javascript') { // strip out the href return ['a', mergeAttributes(this.options.HTMLAttributes, { ...HTMLAttributes, href: '' }), 0] }