Skip to content

markdown: escape alt text in images #2478

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.

Sign up for GitHub

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

Merged
srawlins merged 3 commits into from
Jul 29, 2026
Merged

markdown: escape alt text in images #2478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dca3eaf
markdown: escape alt text in images
srawlins Jul 23, 2026
e7f4ddb
feedback
srawlins Jul 23, 2026
245f765
simplify
srawlins Jul 28, 2026
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
1 change: 1 addition & 0 deletions pkgs/markdown/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* Optimize email autolink regex parsing in `AutolinkExtensionSyntax` by bounding
quantifiers to RFC limits, improving performance on inputs with long sequences
of dots or alphanumeric characters.
* Escape image description text when assigning it to the `alt` attribute.

## 7.3.1

Expand Down
22 changes: 13 additions & 9 deletions pkgs/markdown/lib/src/inline_syntaxes/image_syntax.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,19 @@ class ImageSyntax extends LinkSyntax {
element.attributes['src'] = normalizeLinkDestination(
escapePunctuation(destination),
);
element.attributes['alt'] = children.map((node) {
// See https://spec.commonmark.org/0.30/#image-description.
// An image description may contain links. Fetch text from the alt
// attribute if this nested link is an image.
if (node is Element && node.tag == 'img') {
return node.attributes['alt'];
}
return node.textContent;
}).join();
final alt = children
.map((node) {
// See https://spec.commonmark.org/0.30/#image-description.
// An image description may contain links. Fetch text from the alt
// attribute if this nested link is an image.
if (node case Element(tag: 'img', attributes: {'alt': final alt})) {
return alt;
}
return node.textContent;
})
.nonNulls
.join();
element.attributes['alt'] = escapeAttributeCharactersValue(alt);
if (title != null && title.isNotEmpty) {
element.attributes['title'] = normalizeLinkTitle(title);
}
Expand Down
11 changes: 11 additions & 0 deletions pkgs/markdown/lib/src/util.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ String escapeHtml(String html, {bool escapeApos = true}) => HtmlEscape(
String escapeHtmlAttribute(String text) =>
const HtmlEscape(HtmlEscapeMode.attribute).convert(text);

/// Escapes raw `"` (double quote), `'` (single quote), `<` (less than) and
/// `>` (greater than) characters.
///
/// Unlike [escapeHtmlAttribute], this does not escape `&` (ampersand) to
/// avoid double-escaping already-escaped HTML entities.
Comment on lines +29 to +30

@natebosch natebosch Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it that we end up with somethign here which is partially escaped? Where would we have escaped & but not these others?

@srawlins srawlins Jul 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is awkward. By the time the alt text has been put into an element's textContent, it has already passed through EscapeHtmlSyntax, emitting escaped Text nodes. So if you have input like ![a & " b](source), then the text that comes out of node.textContent, in the edited lines above, has a &amp; " b.

This escaping is done early because we parse the alt text into more inline nodes, which is allowed by the spec. The spec allows, for example ![foo *bar*](source), such that the description has inline formatting. In HTML the inline formatting is dropped, producing foo bar alt text. But the model is supposed to parse and keep track of that inner parsing (it also might be required for... correctly parsing erroneous text?).

String escapeAttributeCharactersValue(String text) => text
.replaceAll('"', '&quot;')
.replaceAll("'", '&#x27;')
.replaceAll('<', '&lt;')
.replaceAll('>', '&gt;');
Comment thread
kevmoo marked this conversation as resolved.

/// "Normalizes" a link label, according to the [CommonMark spec].
///
/// [CommonMark spec] https://spec.commonmark.org/0.30/#link-label
Expand Down
26 changes: 26 additions & 0 deletions pkgs/markdown/test/markdown_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,32 @@ void main() async {
''',
);

validateCore(
'Image alt attribute breakout XSS',
'![<b x=" onerror=alert(1) y=">](bad)',
'<p><img src="bad" alt="&lt;b x=&quot; onerror=alert(1) y=&quot;&gt;" /></p>\n',
inlineSyntaxes: [InlineHtmlSyntax()],
);

validateCore(
'Image alt attribute escaping ampersand and quotes',
'![a & " b](bad)',
'<p><img src="bad" alt="a &amp; &quot; b" /></p>\n',
);

validateCore(
'Image alt attribute double-encoded entity',
'![&amp;amp;](bad)',
'<p><img src="bad" alt="&amp;amp;" /></p>\n',
);

validateCore(
'Image alt attribute with raw HTML tag under GFM',
'![foo <a> bar](bad)',
'<p><img src="bad" alt="foo &lt;a&gt; bar" /></p>\n',
inlineSyntaxes: [InlineHtmlSyntax()],
);

validateCore(
'Unicode ellipsis as punctuation',
'''
Expand Down
Loading