Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(extension-youtube) XSS risk with src tag #4602

Merged
merged 4 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/extension-link/src/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
},

Expand Down
8 changes: 6 additions & 2 deletions packages/extension-youtube/src/utils.ts
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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
Expand Down
64 changes: 64 additions & 0 deletions tests/cypress/integration/extensions/link.spec.ts
Original file line number Diff line number Diff line change
@@ -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()
})
})
60 changes: 60 additions & 0 deletions tests/cypress/integration/extensions/youtube.spec.ts
Original file line number Diff line number Diff line change
@@ -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()
})
})
})