Repository: marp-team/marp-core (ts)

Issue:
Relax HTML allowlist
[Marp Core's default HTML allowlist is only allowed `<br>` tag.](https://github.com/marp-team/marp-core/blob/d393312a9f6aed070c481cb879f92ccb0e6d1e78/src/marp.ts#L36) It's definitely safe but but this default set is not matching with real use case. A lot of Marp users enjoy using custom HTML tags.

It is better to update default allowlist of HTML to include allowed elements in common Marp slides and Web services. e.g. `<div>`, `<span>`, and HTML elements that are converted by Markdown.

https://github.com/gjtorikian/html-pipeline/blob/HEAD/lib/html/pipeline/sanitization_filter.rb is mentioned as GitHub style sanitization from some famous Markdown parsers. ([remark](https://github.com/remarkjs/remark-html#api), [marked-sanitizer-github](https://github.com/rhysd/marked-sanitizer-github))

Pull request description:
Relax default HTML allowlist
Resolves #301, #368.

Marp Core now becomes to allow a lot of HTML elements and attributes by default, that are considered as safe. The allowlist covers some mentioned elements and attributes in the feedback thread https://github.com/orgs/marp-team/discussions/501.

`href`, `src`, and similar attributes include URL schema checking. By default, non-http and https schemes are always sanitized. If the attribute may allow an image, `data:image/*` is also allowed.

Elements and attributes that have too much power, such as `<script>`, `<link>` element, `on***`, and `style` attribute, are still not allowed by default. The developer can enable them through `html: true` constructor option as before.

> [!TIP]
> To get back into the previous allowlist compliant with Marp Core v3, set `html` constructor option as `html: { br: [] }`. That only allows `<br>` tag.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ab8d8c9..c146256 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -42,6 +42,7 @@
   - Bump markdown-it to [v14.1.0](https://github.com/markdown-it/markdown-it/blob/master/CHANGELOG.md#1410---2024-03-19), and follow the latest spec of [CommonMark 0.31.2](https://spec.commonmark.org/0.31.2/)
   - Support for CSS nesting (`cssNesting` constructor option)
 - Use simpler CSS minification when `minifyCSS` option is enabled ([#381](https://github.com/marp-team/marp-core/pull/381))
+- Relax HTML allowlist: Allowed a lot of HTML elements and attributes by default ([#301](https://github.com/marp-team/marp-core/issues/301), [#383](https://github.com/marp-team/marp-core/pull/383))
 
 * Upgrade development Node.js to v20 LTS ([#359](https://github.com/marp-team/marp-core/pull/359))
 * Upgrade dependent packages to the latest version ([#380](https://github.com/marp-team/marp-core/pull/380))
diff --git a/README.md b/README.md
index 6032f5e..1397904 100644
--- a/README.md
+++ b/README.md
@@ -243,6 +243,7 @@ const marp = new Marp({
 
 Setting whether to render raw HTML in Markdown. It's an alias to `markdown.html` ([markdown-it option](https://markdown-it.github.io/markdown-it/#MarkdownIt.new)) but has additional feature about HTML allowlist.
 
+- (default): Use Marp's default allowlist.
 - `true`: The all HTML will be allowed.
 - `false`: All HTML except supported in Marpit Markdown will be disallowed.
 
@@ -265,7 +266,7 @@ By passing `object`, you can set the allowlist to specify allowed tags and attri
 }
 ```
 
-Marp core allows only `<br>` tag by default. That is defined in [a readonly `html` member in `Marp` class](https://github.com/marp-team/marp-core/blob/38fb33680c5837f9c48d8a88ac94b9f0862ab6c7/src/marp.ts#L34).
+By default, Marp Core allows known HTML elements and attributes that are considered as safe. That is defined as a readonly `html` member in `Marp` class. [See the full default allowlist in the source code.](src/html/allowlist.ts)
 
 > [!NOTE]
 > Whatever any option is selected, `<!-- HTML comment -->` and `<style>` tags are always parsed by Marpit for directives / tweaking style.
diff --git a/marp.config.mjs b/marp.config.mjs
index 27aee53..fe0d90b 100644
--- a/marp.config.mjs
+++ b/marp.config.mjs
@@ -7,7 +7,6 @@ export default {
     path.dirname(new URL(import.meta.url).pathname),
     './sandbox',
   ),
-  html: true,
   options: {
     minifyCSS: false,
   },
diff --git a/src/html/allowlist.ts b/src/html/allowlist.ts
new file mode 100644
index 0000000..a638bc2
--- /dev/null
+++ b/src/html/allowlist.ts
@@ -0,0 +1,242 @@
+export type HTMLAllowList = {
+  [tag: string]:
+    | string[]
+    | { [attr: string]: boolean | ((value: string) => string) }
+}
+
+const globalAttrs = {
+  class: true,
+  dir: (value) => {
+    const normalized = value.toLowerCase()
+    return ['rtl', 'ltr', 'auto'].includes(normalized) ? normalized : ''
+  },
+  lang: true,
+  title: true,
+} as const satisfies HTMLAllowList[string]
+
+const generateUrlSanitizer =
+  (schemas: string[]) =>
+  (value: string): string => {
+    if (value.includes(':')) {
+      // Check the URL schema if it exists
+      const trimmed = value.trim().toLowerCase()
+      const schema = trimmed.split(':', 1)[0]
+
+      for (const allowedSchema of schemas) {
+        if (schema === allowedSchema) return value
+        if (allowedSchema.includes(':') && trimmed.startsWith(allowedSchema))
+          return value
+      }
+
+      return ''
+    }
+    return value
+  }
+
+const webUrlSanitizer = generateUrlSanitizer(['http', 'https'])
+const imageUrlSanitizer = generateUrlSanitizer(['http', 'https', 'data:image/'])
+const srcSetSanitizer = (value: string): string => {
+  for (const src of value.split(',')) {
+    if (!imageUrlSanitizer(src)) return ''
+  }
+  return value
+}
+
+export const defaultHTMLAllowList = {
+  a: {
+    ...globalAttrs,
+    href: webUrlSanitizer,
+    name: true, // deprecated attribute, but still useful in Marp for making stable anchor link
+    rel: true,
+    target: true,
+  },
+  abbr: globalAttrs,
+  address: globalAttrs,
+  article: globalAttrs,
+  aside: globalAttrs,
+  audio: {
+    ...globalAttrs,
+    autoplay: true,
+    controls: true,
+    loop: true,
+    muted: true,
+    preload: true,
+    src: webUrlSanitizer,
+  },
+  b: globalAttrs,
+  bdi: globalAttrs,
+  bdo: globalAttrs,
+  big: globalAttrs,
+  blockquote: {
+    ...globalAttrs,
+    cite: webUrlSanitizer,
+  },
+  br: globalAttrs,
+  caption: globalAttrs,
+  center: globalAttrs, // deprecated
+  cite: globalAttrs,
+  code: globalAttrs,
+  col: {
+    ...globalAttrs,
+    align: true,
+    valign: true,
+    span: true,
+    width: true,
+  },
+  colgroup: {
+    ...globalAttrs,
+    align: true,
+    valign: true,
+    span: true,
+    width: true,
+  },
+  dd: globalAttrs,
+  del: {
+    ...globalAttrs,
+    cite: webUrlSanitizer,
+    datetime: true,
+  },
+  details: {
+    ...globalAttrs,
+    open: true,
+  },
+  div: globalAttrs,
+  dl: globalAttrs,
+  dt: globalAttrs,
+  em: globalAttrs,
+  figcaption: globalAttrs,
+  figure: globalAttrs,
+  // footer: globalAttrs, // Inserted by Marpit directives so disallowed to avoid confusion
+  h1: globalAttrs,
+  h2: globalAttrs,
+  h3: globalAttrs,
+  h4: globalAttrs,
+  h5: globalAttrs,
+  h6: globalAttrs,
+  // header: globalAttrs, // Inserted by Marpit directives so disallowed to avoid confusion
+  hr: globalAttrs,
+  i: globalAttrs,
+  img: {
+    ...globalAttrs,
+    align: true, // deprecated attribute, but still useful in Marp for aligning image
+    alt: true,
+    decoding: true,
+    height: true,
+    loading: true,
+    src: imageUrlSanitizer,
+    srcset: srcSetSanitizer,
+    title: true,
+    width: true,
+  },
+  ins: {
+    ...globalAttrs,
+    cite: webUrlSanitizer,
+    datetime: true,
+  },
+  kbd: globalAttrs,
+  li: {
+    ...globalAttrs,
+    type: true,
+    value: true,
+  },
+  mark: globalAttrs,
+  nav: globalAttrs,
+  ol: {
+    ...globalAttrs,
+    reversed: true,
+    start: true,
+    type: true,
+  },
+  p: globalAttrs,
+  picture: globalAttrs,
+  pre: globalAttrs,
+  source: {
+    height: true,
+    media: true,
+    sizes: true,
+    src: imageUrlSanitizer,
+    srcset: srcSetSanitizer,
+    type: true,
+    width: true,
+  },
+  q: {
+    ...globalAttrs,
+    cite: webUrlSanitizer,
+  },
+  rp: globalAttrs,
+  rt: globalAttrs,
+  ruby: globalAttrs,
+  s: globalAttrs,
+  section: globalAttrs,
+  small: globalAttrs,
+  span: globalAttrs,
+  sub: globalAttrs,
+  summary: globalAttrs,
+  sup: globalAttrs,
+  strong: globalAttrs,
+  strike: globalAttrs,
+  table: {
+    ...globalAttrs,
+    width: true,
+    border: true,
+    align: true,
+    valign: true,
+  },
+  tbody: {
+    ...globalAttrs,
+    align: true,
+    valign: true,
+  },
+  td: {
+    ...globalAttrs,
+    width: true,
+    rowspan: true,
+    colspan: true,
+    align: true,
+    valign: true,
+  },
+  tfoot: {
+    ...globalAttrs,
+    align: true,
+    valign: true,
+  },
+  th: {
+    ...globalAttrs,
+    width: true,
+    rowspan: true,
+    colspan: true,
+    align: true,
+    valign: true,
+  },
+  thead: {
+    ...globalAttrs,
+    align: true,
+    valign: true,
+  },
+  time: {
+    ...globalAttrs,
+    datetime: true,
+  },
+  tr: {
+    ...globalAttrs,
+    rowspan: true,
+    align: true,
+    valign: true,
+  },
+  u: globalAttrs,
+  ul: globalAttrs,
+  video: {
+    ...globalAttrs,
+    autoplay: true,
+    controls: true,
+    loop: true,
+    muted: true,
+    playsinline: true,
+    poster: imageUrlSanitizer,
+    preload: true,
+    src: webUrlSanitizer,
+    height: true,
+    width: true,
+  },
+  wbr: globalAttrs,
+} as const satisfies HTMLAllowList
diff --git a/src/html/html.ts b/src/html/html.ts
index b386421..66c0d71 100644
--- a/src/html/html.ts
+++ b/src/html/html.ts
@@ -1,6 +1,5 @@
 import selfClosingTags from 'self-closing-tags'
-import { FilterXSS } from 'xss'
-import { friendlyAttrValue, escapeAttrValue } from 'xss/lib/default'
+import { FilterXSS, friendlyAttrValue, escapeAttrValue } from 'xss'
 import { MarpOptions } from '../marp'
 
 const selfClosingRegexp = /\s*\/?>$/
@@ -12,7 +11,7 @@ const xhtmlOutFilter = new FilterXSS({
     }
     return html
   },
-  whiteList: {},
+  allowList: {},
 })
 
 export function markdown(md): void {
@@ -60,7 +59,7 @@ export function markdown(md): void {
       }
 
       const filter = new FilterXSS({
-        whiteList: allowList,
+        allowList,
         onIgnoreTag: (_, rawHtml) => (html === true ? rawHtml : undefined),
         safeAttrValue: (tag, attr, value) => {
           let ret = friendlyAttrValue(value)
diff --git a/src/marp.ts b/src/marp.ts
index f10b787..883af7b 100644
--- a/src/marp.ts
+++ b/src/marp.ts
@@ -9,6 +9,7 @@ import * as autoScalingPlugin from './auto-scaling'
 import * as customElements from './custom-elements'
 import * as emojiPlugin from './emoji/emoji'
 import { generateHighlightJSInstance } from './highlightjs'
+import { defaultHTMLAllowList, type HTMLAllowList } from './html/allowlist'
 import * as htmlPlugin from './html/html'
 import * as mathPlugin from './math/math'
 import * as scriptPlugin from './script/script'
@@ -17,13 +18,7 @@ import * as slugPlugin from './slug/slug'
 
 export interface MarpOptions extends Options {
   emoji?: emojiPlugin.EmojiOptions
-  html?:
-    | boolean
-    | {
-        [tag: string]:
-          | string[]
-          | { [attr: string]: boolean | ((value: string) => string) }
-      }
+  html?: boolean | HTMLAllowList
   markdown?: object
   math?: mathPlugin.MathOptions
   minifyCSS?: boolean
@@ -36,7 +31,7 @@ export class Marp extends Marpit {
 
   private _highlightjs: HLJSApi | undefined
 
-  static readonly html = { br: [] }
+  static readonly html = defaultHTMLAllowList
 
   constructor(opts: MarpOptions = {}) {
     const mdOpts: Record<string, any> = {


Test patch:
diff --git a/test/marp.ts b/test/marp.ts
index bfeb712..42d14f3 100644
--- a/test/marp.ts
+++ b/test/marp.ts
@@ -228,14 +228,140 @@ describe('Marp', () => {
 
   describe('html option', () => {
     describe('with default option', () => {
-      it('sanitizes HTML tag by default', () => {
-        const { html } = marp().render('<b>abc</b>')
-        expect(load(html)('b')).toHaveLength(0)
+      it('allows known HTML tags by default', () => {
+        const $b = load(marp().render('<b>abc</b>').html)
+        expect($b('b')).toHaveLength(1)
+
+        const $span = load(marp().render('<span>abc</span>').html)
+        expect($span('span')).toHaveLength(1)
+
+        const $div = load(marp().render('<div class="test">abc</div>').html)
+        expect($div('div.test')).toHaveLength(1)
+
+        const $br = load(marp().render('allow<br>break').html)
+        expect($br('br')).toHaveLength(1)
+
+        const $aHref = load(
+          marp().render('<a href="https://example.com/">link</a>').html,
+        )
+        expect($aHref('a[href="https://example.com/"]')).toHaveLength(1)
+
+        const $img = load(
+          marp().render(
+            '<img src="https://example.com/hello.jpg" alt="Hello" />',
+          ).html,
+        )
+        expect(
+          $img('img[src="https://example.com/hello.jpg"][alt="Hello"]'),
+        ).toHaveLength(1)
+
+        const $imgLocal = load(
+          marp().render('<img src="./hello.jpg" alt="Hello" />').html,
+        )
+        expect($imgLocal('img[src="./hello.jpg"][alt="Hello"]')).toHaveLength(1)
+
+        const $imgAbsolute = load(
+          marp().render('<img src="/hello.jpg" alt="Hello" />').html,
+        )
+        expect($imgAbsolute('img[src="/hello.jpg"][alt="Hello"]')).toHaveLength(
+          1,
+        )
+
+        const $imgSameSchema = load(
+          marp().render('<img src="//example.com/hello.jpg" alt="Hello" />')
+            .html,
+        )
+        expect(
+          $imgSameSchema('img[src="//example.com/hello.jpg"][alt="Hello"]'),
+        ).toHaveLength(1)
+
+        const $imgData = load(
+          marp().render(
+            `<img src="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg'/%3E" alt="Hello" />`,
+          ).html,
+        )
+        expect(
+          $imgData('img[src^="data:image/svg+xml"][alt="Hello"]'),
+        ).toHaveLength(1)
+
+        const $imgSrcSet = load(
+          marp().render(
+            '<img src="hello.jpg" alt="Hello" srcset="hello@2x.jpg 2x, data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7 3x" />',
+          ).html,
+        )
+        expect(
+          $imgSrcSet(
+            'img[src="hello.jpg"][srcset="hello@2x.jpg 2x, data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7 3x"]',
+          ),
+        ).toHaveLength(1)
+
+        const $pDir = load(marp().render('<p dir="ltr">ltr</p>').html)
+        expect($pDir('p[dir="ltr"]')).toHaveLength(1)
+
+        const $pDirNormalize = load(marp().render('<p dir="LTR">ltr</p>').html)
+        expect($pDirNormalize('p[dir="ltr"]')).toHaveLength(1)
       })
 
-      it('allows <br> tag', () => {
-        const { html } = marp().render('allow<br>break')
-        expect(load(html)('br')).toHaveLength(1)
+      it('does not allow some insecure and invalid elements/attributes', () => {
+        // Insecure elements
+        expect(
+          load(
+            marp({ script: false }).render('<script>alert(1)</script>').html,
+          )('script'),
+        ).toHaveLength(0)
+        expect(load(marp().render('<input />').html)('input')).toHaveLength(0)
+        expect(load(marp().render('<button />').html)('button')).toHaveLength(0)
+
+        // Insecure attributes
+        const $onclick = load(
+          marp().render('<a href="#" onclick="alert(1)">test</a>').html,
+        )
+        expect($onclick('a')).toHaveLength(1)
+        expect($onclick('a[onclick]')).toHaveLength(0)
+
+        const $javascriptSchema = load(
+          marp().render('<a href="javascript:alert(1)">test</a>').html,
+        )
+        expect($javascriptSchema('a[href]')).toHaveLength(1)
+        expect($javascriptSchema('a[href^="javascript:"]')).toHaveLength(0)
+
+        const $dataSchema = load(
+          marp().render(
+            "<a href='data:text/html,<script>alert(1)</script>'>test</a>",
+          ).html,
+        )
+        expect($dataSchema('a[href]')).toHaveLength(1)
+        expect($dataSchema('a[href^="data:text/html"]')).toHaveLength(0)
+
+        // Invalid schema
+        const $ftpSchema = load(
+          marp().render('<a href="ftp://example.com">test</a>').html,
+        )
+        expect($ftpSchema('a[href]')).toHaveLength(1)
+        expect($ftpSchema('a[href^="ftp:"]')).toHaveLength(0)
+
+        const $mailtoSchema = load(
+          marp().render("<a href='mailto:test@example.com'>mail</a>").html,
+        )
+        expect($mailtoSchema('a[href]')).toHaveLength(1)
+        expect($mailtoSchema('a[href^="mailto:"]')).toHaveLength(0)
+
+        const $imgSrcSet = load(
+          marp().render(
+            '<img src="hello.jpg" alt="Hello" srcset="hello@2x.jpg 2x, unknown:unknown.jpg 2x" />',
+          ).html,
+        )
+        expect($imgSrcSet('img[src="hello.jpg"]')).toHaveLength(1)
+        expect(
+          $imgSrcSet('img[src="hello.jpg"][srcset*="unknown"]'),
+        ).toHaveLength(0)
+
+        // Invalid attributes
+        const $pDirUnknown = load(
+          marp().render('<p dir="unknown">unknown</p>').html,
+        )
+        expect($pDirUnknown('p[dir]')).toHaveLength(1)
+        expect($pDirUnknown('p[dir="unknown"]')).toHaveLength(0)
       })
 
       it('renders void element with normalized', () => {
@@ -243,13 +369,21 @@ describe('Marp', () => {
         expect(marp().render('<br  >').html).toContain('<br />')
         expect(marp().render('<br/>').html).toContain('<br />')
         expect(marp().render('<br />').html).toContain('<br />')
-        expect(marp().render('<br class="sanitize">').html).toContain('<br />')
+        expect(marp().render('<br class="sanitize">').html).toContain(
+          '<br class="sanitize" />',
+        )
+        expect(marp().render("<br class='sanitize'>").html).toContain(
+          '<br class="sanitize" />',
+        )
+        expect(marp().render(`<br class='"sanitize"'>`).html).toContain(
+          '<br class="&quot;sanitize&quot;" />',
+        )
         expect(marp().render('<br></br>').html).toContain('<br /><br />')
         expect(marp().render('<BR >').html).toContain('<br />')
       })
 
-      // https://github.com/yhatt/marp/issues/243
       it('does not sanitize header and footer', () => {
+        // https://github.com/yhatt/marp/issues/243
         const markdown = '<!--\nheader: "**header**"\nfooter: "*footer*"\n-->'
         const $ = load(marp().render(markdown).html)
 
@@ -311,7 +445,7 @@ function matchwo(a,b)
     describe('with true', () => {
       const m = marp({ html: true })
 
-      it('allows HTML tag', () => {
+      it('allows any HTML tag', () => {
         const { html } = m.render('<b data-custom="test">abc</b>')
         expect(load(html)('b[data-custom="test"]')).toHaveLength(1)
       })
@@ -323,14 +457,16 @@ function matchwo(a,b)
         expect(m.render('<br />').html).toContain('<br />')
         expect(m.render('<br></br>').html).toContain('<br /><br />')
         expect(m.render('<BR >').html).toContain('<br />')
-        expect(m.render('<br  class="normalize">').html).toContain(
-          '<br class="normalize" />',
+
+        // Pass through quotes for attributes
+        expect(m.render("<br  class='normalize'>").html).toContain(
+          "<br class='normalize' />",
         )
       })
     })
 
     describe('with false', () => {
-      it('sanitizes <br> tag', () => {
+      it('sanitizes tags', () => {
         const { html } = marp({ html: false }).render('sanitize<br>break')
         expect(load(html)('br')).toHaveLength(0)
       })
@@ -389,7 +525,12 @@ function matchwo(a,b)
       it('does not normalize void element', () => {
         expect(m.render('<br>').html).toContain('<br>')
         expect(m.render('<br />').html).toContain('<br />')
-        expect(m.render('<br class="sanitize">').html).toContain('<br>')
+        expect(m.render('<br class="sanitize">').html).toContain(
+          '<br class="sanitize">',
+        )
+        expect(m.render("<br class='sanitize'>").html).toContain(
+          '<br class="sanitize">',
+        )
         expect(m.render('<br></br>').html).toContain('<br></br>')
       })
     })