Merge pull request #4602 from C-Hess/fix/xss-youtube-risk

fix(extension-youtube) XSS risk with src tag
ueberdosis · Nov 20, 2023 · b2e3b7e · b2e3b7e
2 parents acbf47e + e6947ba
commit b2e3b7e
Show file tree

Hide file tree

Showing 4 changed files with 136 additions and 2 deletions.
diff --git a/packages/extension-link/src/link.ts b/packages/extension-link/src/link.ts
@@ -121,6 +121,12 @@ export const Link = Mark.create<LinkOptions>({
   },
 
   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:')) {
+      // strip out the href
+      return ['a', mergeAttributes(this.options.HTMLAttributes, { ...HTMLAttributes, href: '' }), 0]
+    }
     return ['a', mergeAttributes(this.options.HTMLAttributes, HTMLAttributes), 0]
   },
 

diff --git a/packages/extension-youtube/src/utils.ts b/packages/extension-youtube/src/utils.ts
@@ -1,5 +1,5 @@
-export const YOUTUBE_REGEX = /^(https?:\/\/)?(www\.|music\.)?(youtube\.com|youtu\.be)(?!.*\/channel\/)(?!\/@)(.+)?$/
-export const YOUTUBE_REGEX_GLOBAL = /^(https?:\/\/)?(www\.|music\.)?(youtube\.com|youtu\.be)(?!.*\/channel\/)(?!\/@)(.+)?$/g
+export const YOUTUBE_REGEX = /^(https?:\/\/)?(www\.|music\.)?(youtube\.com|youtu\.be)\/(?!channel\/)(?!@)(.+)?$/
+export const YOUTUBE_REGEX_GLOBAL = /^(https?:\/\/)?(www\.|music\.)?(youtube\.com|youtu\.be)\/(?!channel\/)(?!@)(.+)?$/g
 
 export const isValidYoutubeUrl = (url: string) => {
   return url.match(YOUTUBE_REGEX)
@@ -52,6 +52,10 @@ export const getEmbedUrlFromYoutubeUrl = (options: GetEmbedUrlOptions) => {
     startAt,
   } = options
 
+  if (!isValidYoutubeUrl(url)) {
+    return null
+  }
+
   // if is already an embed url, return it
   if (url.includes('/embed/')) {
     return url

diff --git a/tests/cypress/integration/extensions/link.spec.ts b/tests/cypress/integration/extensions/link.spec.ts
@@ -0,0 +1,64 @@
+import { Editor } from '@tiptap/core'
+import Document from '@tiptap/extension-document'
+import Link from '@tiptap/extension-link'
+import Paragraph from '@tiptap/extension-paragraph'
+import Text from '@tiptap/extension-text'
+
+/**
+ * Most link tests should actually exist in the demo/ app folder
+ */
+describe('extension-link', () => {
+  const editorElClass = 'tiptap'
+  let editor: Editor | null = null
+
+  const createEditorEl = () => {
+    const editorEl = document.createElement('div')
+
+    editorEl.classList.add(editorElClass)
+    document.body.appendChild(editorEl)
+    return editorEl
+  }
+  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)',
+                    },
+                  },
+                ],
+              },
+            ],
+          },
+        ],
+      },
+    })
+
+    // eslint-disable-next-line no-script-url
+    expect(editor.getHTML()).to.not.include('javascript:alert(window.origin)')
+
+    editor?.destroy()
+    getEditorEl()?.remove()
+  })
+})
diff --git a/tests/cypress/integration/extensions/youtube.spec.ts b/tests/cypress/integration/extensions/youtube.spec.ts
@@ -0,0 +1,60 @@
+import { Editor } from '@tiptap/core'
+import Document from '@tiptap/extension-document'
+import Paragraph from '@tiptap/extension-paragraph'
+import Text from '@tiptap/extension-text'
+import Youtube from '@tiptap/extension-youtube'
+
+/**
+ * Most youtube tests should actually exist in the demo/ app folder
+ */
+describe('extension-youtube', () => {
+  const editorElClass = 'tiptap'
+  let editor: Editor | null = null
+
+  const createEditorEl = () => {
+    const editorEl = document.createElement('div')
+
+    editorEl.classList.add(editorElClass)
+    document.body.appendChild(editorEl)
+    return editorEl
+  }
+  const getEditorEl = () => document.querySelector(`.${editorElClass}`)
+
+  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)//embed/',
+    'https://youtube.google.com/embed/fdsafsdf',
+    'https://youtube.com.bad/embed',
+  ]
+
+  invalidUrls.forEach(url => {
+    it(`does not output html for javascript schema or non-youtube links for url ${url}`, () => {
+      editor = new Editor({
+        element: createEditorEl(),
+        extensions: [
+          Document,
+          Text,
+          Paragraph,
+          Youtube,
+        ],
+        content: {
+          type: 'doc',
+          content: [
+            {
+              type: 'youtube',
+              attrs: {
+                src: url,
+              },
+            },
+          ],
+        },
+      })
+
+      expect(editor.getHTML()).to.not.include(url)
+
+      editor?.destroy()
+      getEditorEl()?.remove()
+    })
+  })
+})