facebook / facebook/astryx

Markdown: parser's isSafeUrl documents rejecting data: but only rejects data:text/html

Open
#6,323 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
13k
Forks
1.1k
Avg merge
1d 15h
Merged PRs (30d)
690

Description

### Summary

`@astryxdesign/core/Markdown` applies two different URL checks at two stages, and the weaker one guards the public parser API.

| Stage | Function | Rejects |
|---|---|---|
| Parser (`parser.js`) | `isSafeUrl` | `javascript:`, `vbscript:`, `data:text/html` |
| Renderer (`Markdown.js`) | `sanitizeUrl` | `/^(javascript\|data\|vbscript):/i` |

The renderer is the stricter of the two and blocks all `data:`. **The `` component itself is not affected** — it never emits a `data:` href or src, and on rejection it falls back to plain text without calling the `components` override.

The issue is the parser, which is a public export.

### Doc/implementation mismatch

`isSafeUrl`'s doc comment reads:

> Reject URLs with dangerous schemes (javascript:, vbscript:, data:) that could execute arbitrary code when rendered as link hrefs or image srcs.

It names `data:`, but the implementation tests only `data:text/html`.

### Why it matters

`parseMarkdown`, `parseMarkdownIncremental`, `parseInline` and `createIncrementalState` are public exports of `@astryxdesign/core/Markdown`. A consumer building a custom renderer over the AST receives `data:` URLs in `link.href` and `image.src`, and would reasonably read that doc comment as meaning the scheme check had already been applied.

### Reproduction (0.5.4)

```js
import { parseMarkdown } from '@astryxdesign/core/Markdown'

parseMarkdown('[x](data:image/svg+xml,

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.