From dda0033cd49449572d077bbecd33b18d8d05f48a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krassowski?= <5832902+krassowski@users.noreply.github.com> Date: Fri, 19 Jan 2024 12:43:25 +0000 Subject: [PATCH] Merge pull request from GHSA-4m77-cmpx-vjc4 * Fix sanitization and escaping for MD headings in ToC * Add a unit test for `getHeadingId` * Make `sanitizer` argument optional in getHeadingId, integrity update (cherry picked from commit e1b3aabab603878e46add445a3114e838411d2df) --- .../markdownviewer-extension/src/index.ts | 16 +++++++--- packages/markdownviewer/src/toc.ts | 10 ++++--- packages/notebook/src/toc.ts | 14 ++++++--- packages/notebook/src/widget.ts | 5 ++-- packages/rendermime-interfaces/src/index.ts | 4 +-- packages/toc/package.json | 1 + packages/toc/src/utils/markdown.ts | 30 ++++++++++++------- packages/toc/test/markdown.spec.ts | 24 +++++++++++++++ packages/toc/tsconfig.json | 3 ++ packages/toc/tsconfig.test.json | 3 ++ yarn.lock | 1 + 11 files changed, 85 insertions(+), 26 deletions(-) diff --git a/packages/markdownviewer-extension/src/index.ts b/packages/markdownviewer-extension/src/index.ts index 206dde5c47f2..759aa710696f 100644 --- a/packages/markdownviewer-extension/src/index.ts +++ b/packages/markdownviewer-extension/src/index.ts @@ -10,7 +10,7 @@ import { JupyterFrontEnd, JupyterFrontEndPlugin } from '@jupyterlab/application'; -import { WidgetTracker } from '@jupyterlab/apputils'; +import { ISanitizer, WidgetTracker } from '@jupyterlab/apputils'; import { PathExt } from '@jupyterlab/coreutils'; import { IMarkdownViewerTracker, @@ -20,6 +20,7 @@ import { MarkdownViewerTableOfContentsFactory } from '@jupyterlab/markdownviewer'; import { + IRenderMime, IRenderMimeRegistry, markdownRendererFactory } from '@jupyterlab/rendermime'; @@ -49,7 +50,12 @@ const plugin: JupyterFrontEndPlugin = { description: 'Adds markdown file viewer and provides its tracker.', provides: IMarkdownViewerTracker, requires: [IRenderMimeRegistry, ITranslator], - optional: [ILayoutRestorer, ISettingRegistry, ITableOfContentsRegistry], + optional: [ + ILayoutRestorer, + ISettingRegistry, + ITableOfContentsRegistry, + ISanitizer + ], autoStart: true }; @@ -62,7 +68,8 @@ function activate( translator: ITranslator, restorer: ILayoutRestorer | null, settingRegistry: ISettingRegistry | null, - tocRegistry: ITableOfContentsRegistry | null + tocRegistry: ITableOfContentsRegistry | null, + sanitizer: IRenderMime.ISanitizer | null ): IMarkdownViewerTracker { const trans = translator.load('jupyterlab'); const { commands, docRegistry } = app; @@ -182,7 +189,8 @@ function activate( tocRegistry.add( new MarkdownViewerTableOfContentsFactory( tracker, - rendermime.markdownParser + rendermime.markdownParser, + sanitizer ?? rendermime.sanitizer ) ); } diff --git a/packages/markdownviewer/src/toc.ts b/packages/markdownviewer/src/toc.ts index 3873b3b62474..67e2ccab3f78 100644 --- a/packages/markdownviewer/src/toc.ts +++ b/packages/markdownviewer/src/toc.ts @@ -2,7 +2,7 @@ // Distributed under the terms of the Modified BSD License. import { IWidgetTracker } from '@jupyterlab/apputils'; -import { IMarkdownParser } from '@jupyterlab/rendermime'; +import { IMarkdownParser, IRenderMime } from '@jupyterlab/rendermime'; import { TableOfContents, TableOfContentsFactory, @@ -96,7 +96,8 @@ export class MarkdownViewerTableOfContentsFactory extends TableOfContentsFactory */ constructor( tracker: IWidgetTracker, - protected parser: IMarkdownParser | null + protected parser: IMarkdownParser | null, + protected sanitizer: IRenderMime.ISanitizer ) { super(tracker); } @@ -165,13 +166,14 @@ export class MarkdownViewerTableOfContentsFactory extends TableOfContentsFactory const elementId = await TableOfContentsUtils.Markdown.getHeadingId( this.parser!, heading.raw, - heading.level + heading.level, + this.sanitizer ); if (!elementId) { return; } - const selector = `h${heading.level}[id="${elementId}"]`; + const selector = `h${heading.level}[id="${CSS.escape(elementId)}"]`; headingToElement.set( heading, diff --git a/packages/notebook/src/toc.ts b/packages/notebook/src/toc.ts index 5be26426cb0c..94ad80e18af8 100644 --- a/packages/notebook/src/toc.ts +++ b/packages/notebook/src/toc.ts @@ -588,10 +588,14 @@ export class NotebookToCFactory extends TableOfContentsFactory { const findHeadingElement = (cell: Cell): void => { model.getCellHeadings(cell).forEach(async heading => { - const elementId = await getIdForHeading(heading, this.parser!); + const elementId = await getIdForHeading( + heading, + this.parser!, + this.sanitizer + ); const selector = elementId - ? `h${heading.level}[id="${elementId}"]` + ? `h${heading.level}[id="${CSS.escape(elementId)}"]` : `h${heading.level}`; if (heading.outputIndex !== undefined) { @@ -710,7 +714,8 @@ export class NotebookToCFactory extends TableOfContentsFactory { */ export async function getIdForHeading( heading: INotebookHeading, - parser: IRenderMime.IMarkdownParser + parser: IRenderMime.IMarkdownParser, + sanitizer: IRenderMime.ISanitizer ) { let elementId: string | null = null; if (heading.type === Cell.HeadingType.Markdown) { @@ -718,7 +723,8 @@ export async function getIdForHeading( parser, // Type from TableOfContentsUtils.Markdown.IMarkdownHeading (heading as any).raw, - heading.level + heading.level, + sanitizer ); } else if (heading.type === Cell.HeadingType.HTML) { // Type from TableOfContentsUtils.IHTMLHeading diff --git a/packages/notebook/src/widget.ts b/packages/notebook/src/widget.ts index ba24e9e2683f..f5a7dae6797f 100644 --- a/packages/notebook/src/widget.ts +++ b/packages/notebook/src/widget.ts @@ -2191,14 +2191,15 @@ export class Notebook extends StaticNotebook { id = await TableOfContentsUtils.Markdown.getHeadingId( this.rendermime.markdownParser!, mdHeading.raw, - mdHeading.level + mdHeading.level, + this.rendermime.sanitizer ); } break; } if (id === queryId) { const element = this.node.querySelector( - `h${heading.level}[id="${id}"]` + `h${heading.level}[id="${CSS.escape(id)}"]` ) as HTMLElement; return { diff --git a/packages/rendermime-interfaces/src/index.ts b/packages/rendermime-interfaces/src/index.ts index e2558b1bce4a..4a5a41baeccc 100644 --- a/packages/rendermime-interfaces/src/index.ts +++ b/packages/rendermime-interfaces/src/index.ts @@ -486,10 +486,10 @@ export namespace IRenderMime { */ export interface IMarkdownParser { /** - * Render a markdown source. + * Render a markdown source into unsanitized HTML. * * @param source - The string to render. - * @returns - A promise of the string. + * @returns - A promise of the string containing HTML which may require sanitization. */ render(source: string): Promise; } diff --git a/packages/toc/package.json b/packages/toc/package.json index 27234a111a36..f5fb483157cb 100644 --- a/packages/toc/package.json +++ b/packages/toc/package.json @@ -46,6 +46,7 @@ "@jupyterlab/docregistry": "^4.0.10", "@jupyterlab/observables": "^5.0.10", "@jupyterlab/rendermime": "^4.0.10", + "@jupyterlab/rendermime-interfaces": "^3.8.10", "@jupyterlab/translation": "^4.0.10", "@jupyterlab/ui-components": "^4.0.10", "@lumino/coreutils": "^2.1.2", diff --git a/packages/toc/src/utils/markdown.ts b/packages/toc/src/utils/markdown.ts index 603d4473cf13..92b45cc33d5e 100644 --- a/packages/toc/src/utils/markdown.ts +++ b/packages/toc/src/utils/markdown.ts @@ -1,7 +1,9 @@ // Copyright (c) Jupyter Development Team. // Distributed under the terms of the Modified BSD License. +import { Sanitizer } from '@jupyterlab/apputils'; import { IMarkdownParser, renderMarkdown } from '@jupyterlab/rendermime'; +import { IRenderMime } from '@jupyterlab/rendermime-interfaces'; import { TableOfContents } from '../tokens'; /** @@ -24,27 +26,35 @@ export interface IMarkdownHeading extends TableOfContents.IHeading { * * @param raw Raw markdown heading * @param level Heading level + * @param sanitizer HTML sanitizer */ export async function getHeadingId( - parser: IMarkdownParser, + markdownParser: IMarkdownParser, raw: string, - level: number + level: number, + sanitizer?: IRenderMime.ISanitizer ): Promise { try { - const innerHTML = await parser.render(raw); + const host = document.createElement('div'); - if (!innerHTML) { - return null; - } + await renderMarkdown({ + markdownParser, + host, + source: raw, + trusted: false, + sanitizer: sanitizer ?? new Sanitizer(), + shouldTypeset: false, + resolver: null, + linkHandler: null, + latexTypesetter: null + }); - const container = document.createElement('div'); - container.innerHTML = innerHTML; - const header = container.querySelector(`h${level}`); + const header = host.querySelector(`h${level}`); if (!header) { return null; } - return renderMarkdown.createHeaderId(header); + return header.id; } catch (reason) { console.error('Failed to parse a heading.', reason); } diff --git a/packages/toc/test/markdown.spec.ts b/packages/toc/test/markdown.spec.ts index af0592d948cd..d8cebcefcc66 100644 --- a/packages/toc/test/markdown.spec.ts +++ b/packages/toc/test/markdown.spec.ts @@ -2,9 +2,33 @@ // Distributed under the terms of the Modified BSD License. import { TableOfContentsUtils } from '@jupyterlab/toc'; +import { Sanitizer } from '@jupyterlab/apputils'; +import { createMarkdownParser } from '@jupyterlab/markedparser-extension'; +import { IMarkdownParser } from '@jupyterlab/rendermime'; +import { + EditorLanguageRegistry, + IEditorLanguageRegistry +} from '@jupyterlab/codemirror'; describe('TableOfContentsUtils', () => { describe('Markdown', () => { + describe('#getHeadingId', () => { + const languages: IEditorLanguageRegistry = new EditorLanguageRegistry(); + const parser: IMarkdownParser = createMarkdownParser(languages); + const sanitizer = new Sanitizer(); + it.each<[string, string]>([ + ['# Title', 'Title'], + [`# test'">test {#'">}`, `test'\">test-{#'\">}`] + ])('should derive ID from markdown', async (markdown, expectedId) => { + const headingId = await TableOfContentsUtils.Markdown.getHeadingId( + parser, + markdown, + 1, + sanitizer + ); + expect(headingId).toEqual(expectedId); + }); + }); describe('#getHeadings', () => { it.each<[string, TableOfContentsUtils.Markdown.IMarkdownHeading[]]>([ [ diff --git a/packages/toc/tsconfig.json b/packages/toc/tsconfig.json index d2a5e05732cc..bc9463e7c355 100644 --- a/packages/toc/tsconfig.json +++ b/packages/toc/tsconfig.json @@ -26,6 +26,9 @@ }, { "path": "../ui-components" + }, + { + "path": "../rendermime-interfaces" } ] } diff --git a/packages/toc/tsconfig.test.json b/packages/toc/tsconfig.test.json index c74d05d1b8e3..d1c57d511b52 100644 --- a/packages/toc/tsconfig.test.json +++ b/packages/toc/tsconfig.test.json @@ -26,6 +26,9 @@ { "path": "../ui-components" }, + { + "path": "../rendermime-interfaces" + }, { "path": "." }, diff --git a/yarn.lock b/yarn.lock index a428397b3520..874c44d067ed 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4757,6 +4757,7 @@ __metadata: "@jupyterlab/docregistry": ^4.0.10 "@jupyterlab/observables": ^5.0.10 "@jupyterlab/rendermime": ^4.0.10 + "@jupyterlab/rendermime-interfaces": ^3.8.10 "@jupyterlab/testing": ^4.0.10 "@jupyterlab/translation": ^4.0.10 "@jupyterlab/ui-components": ^4.0.10