Skip to content

Commit

Permalink
Merge pull request #4371 from serlo/xss-link-href
Browse files Browse the repository at this point in the history
fix(link-renderer): add guard against injecting javascript in link elements through href
  • Loading branch information
LarsTheGlidingSquirrel authored Dec 20, 2024
2 parents c9628ad + 9df0c8c commit 28a02bc
Show file tree
Hide file tree
Showing 28 changed files with 235 additions and 7 deletions.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
"graphql-request": "^7.1.2",
"iframe-resizer": "^4.4.5",
"io-ts": "^2.2.21",
"isomorphic-dompurify": "^2.19.0",
"js-cookie": "^3.0.5",
"json-diff": "^1.0.6",
"katex": "^0.16.11",
Expand Down
3 changes: 2 additions & 1 deletion apps/web/src/serlo-editor-integration/create-renderers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import type {
EditorDropzoneImageDocument,
EditorInteractiveVideoDocument,
} from '@editor/types/editor-plugins'
import { sanitizeHref } from '@editor/utils/sanitize-href'
import dynamic from 'next/dynamic'
import { ComponentProps } from 'react'

Expand Down Expand Up @@ -262,7 +263,7 @@ export function createRenderers(): InitRenderersArgs {
linkRenderer: ({ href, children }: ComponentProps<LinkRenderer>) => {
return (
<>
<Link href={href}>{children}</Link>
<Link href={sanitizeHref(href)}>{children}</Link>
<ExtraInfoIfRevisionView>{href}</ExtraInfoIfRevisionView>
</>
)
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"@open-iframe-resizer/react": "1.2.1",
"@serlo/katex-styles": "1.0.1",
"@vidstack/react": "next",
"dompurify": "^3.2.0",
"isomorphic-dompurify": "^2.19.0",
"lit": "^3.2.1",
"motion": "^11.11.17",
"react": "^18.2.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/editor/src/editor-integration/create-renderers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { TextAreaExerciseStaticRenderer } from '@editor/plugins/text-area-exerci
import { VideoStaticRenderer } from '@editor/plugins/video/static'
import { EditorPluginType } from '@editor/types/editor-plugin-type'
import { TemplatePluginType } from '@editor/types/template-plugin-type'
import { sanitizeHref } from '@editor/utils/sanitize-href'
import { ComponentProps } from 'react'

export function createRenderers(): InitRenderersArgs {
Expand Down Expand Up @@ -127,7 +128,7 @@ export function createRenderers(): InitRenderersArgs {
return (
<a
className="serlo-link cursor-pointer"
href={href}
href={sanitizeHref(href)}
target="_blank"
rel="noopener noreferrer"
>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { sanitizeHref } from '@editor/utils/sanitize-href'
import { createElement, useCallback } from 'react'
import { RenderElementProps, RenderLeafProps } from 'slate-react'

Expand Down Expand Up @@ -29,7 +30,7 @@ export const useSlateRenderHandlers = ({
if (element.type === 'a') {
return (
<a
href={element.href}
href={sanitizeHref(element.href)}
className="serlo-link cursor-pointer"
{...attributes}
>
Expand Down
11 changes: 11 additions & 0 deletions packages/editor/src/utils/sanitize-href.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import DOMPurify from 'isomorphic-dompurify'

export function sanitizeHref(href: string) {
// Users can add links to content in the editor. So, href can be manipulated by the user and could potentially be abused to inject malicious javascript. We use DOMPurify to validate the href.
// Examples:
// - 'javascript:doSomethingBad()' -> invalid
// - 'https://example.com/' -> valid
const isHrefValid = DOMPurify.isValidAttribute('a', 'href', href)

return isHrefValid ? href : ''
}
Loading

0 comments on commit 28a02bc

Please sign in to comment.