# One Byte Of Disagreement

**How we use Codex to find vulnerabilities worked end to end on GHSA-f8fg-pg57-v4j8**

*XSS in* `league/commonmark`*'s AttributesExtension: the* `on*` *event-handler filter bypassed with a single form feed*

## I. Introduction

`league/commonmark` is the PHP ecosystem's Markdown parser. Not *a* parser *the* parser. It's the CommonMark reference implementation for PHP, it backs Laravel's Markdown rendering, Statamic, October CMS, Drupal modules, Symfony bundles, static site generators, comment systems, wikis, and a very long tail of internal tools that let users type formatted text into a box.

![](https://cdn.hashnode.com/uploads/covers/65102b1d5866800ea27ebefa/2f06bcf5-aeef-48ca-b0ee-d18e9241ec4a.png align="center")

**458 million installs. 13 million per month.** Whenever a PHP application turns untrusted user input into HTML, there's a good chance this library is the thing doing it.

That makes its escaping guarantees load-bearing in a way most libraries' aren't. And one of those guarantees is documented in plain language in the `AttributesExtension` docs:

> Attributes starting with `on` (e.g. `onclick` or `onerror`) are capable of executing JavaScript code and are therefore **never allowed by default**.

This is the story of getting `onclick` past that filter with one invisible byte.

**Vulnerable versions:** `league/commonmark` >= 2.7.0, < 2.9.1 (Composer).

*   **Advisory:** [GHSA-f8fg-pg57-v4j8](https://github.com/thephpleague/commonmark/security/advisories/GHSA-f8fg-pg57-v4j8)
    
*   **CWE:** CWE-79 (XSS), CWE-86 (Improper Neutralization of Invalid Characters in Identifiers)
    
*   **Merged fix:** `dfcdf455` released in **2.9.1** on 2026-08-09, the same day the advisory published.
    

Note the affected range. It doesn't start at 1.0 it starts at **2.7.0**, the release that added the `on*` filter in the first place. The vulnerability was introduced by the security fix that was supposed to prevent it. That is the shape this post is about.

## II. Target selection: a fresh security release is the loudest signal in open source

On **2026-08-03**, CommonMark shipped **2.9.0**, a security release fixing five denial-of-service issues and one XSS. The XSS entry read:

> Fixed the unsafe link filter failing to detect dangerous schemes obfuscated with embedded tabs, newlines, or leading control characters (such as `java<TAB>script:`), which allowed the `allow_unsafe_links` protection to be bypassed via `href` and `src` attributes (GHSA-29pj-957v-52mc)

Read that as an attacker and it says four things at once:

1.  **The class is confirmed present.** Control-character obfuscation defeats this library's filters. Not a theory the maintainer just shipped a fix saying so.
    
2.  **The maintainer accepts and ships this class**, fast, with credit. That's the difference between a finding and a wasted week.
    
3.  **The fix has a stated scope**, and the scope is narrow: *attribute values*, specifically `href` and `src`, specifically scheme detection.
    
4.  **Embargoed fixes are written under time pressure.** A fix that lands in a batch of six on release day is a fix aimed at the reported input, not at the class.
    

So the question wrote itself, and it's a one-liner:

> **The fix normalized control characters in attribute *values*. Does anything normalize them in attribute *names*?**

Six days later the answer was an advisory. This is the highest-yield target-selection heuristic I know, and it costs nothing: **subscribe to security releases of libraries you care about, and read every fix as a statement about where the maintainer thinks the boundary is.** The gap between the reported input and the underlying class is where the next finding lives.

Note what the question is *not*. It is not "find vulnerabilities in commonmark."

## III. How we use Codex

### The economics: exhaustiveness, not insight

An agent is not better than you at noticing that something smells wrong. It is better at **not getting bored**.

This bug is a good illustration, because the reasoning is trivial once you see the table, and the table is deathly dull to build by hand: you have to walk a parse pipeline and write down, function by function, the *exact set of bytes* each one treats as whitespace. Nobody does that on a Tuesday. It's five minutes of agent time and it's where the entire finding lives.

So we never ask the model to be clever. We ask it to be **complete**, and we structure every prompt so that incompleteness is visible.

### Setup

**Pin the tree.** `git clone && git checkout 2.9.0`. Every claim is against a tag you can hand a maintainer.

**Feed it the documentation, not just the code.** The docs define what a developer *believes* the security model is. `AttributesExtension`'s docs promise `on*` attributes are "never allowed by default." That sentence is the specification the bug violates and quoting it back in the report is what makes the severity self-evident rather than argued.

**Feed it the fix you're hunting variants of.** Give the agent the 2.9.0 diff as a *premise*, not as proof: "this is what was fixed, and this is exactly how far the fix reaches."

**Keep it read-only during the audit.** An agent that has started writing a patch has committed to the bug being real, and will defend that position for the rest of the session.

### Pattern 1: Inventory: a table, with security talk banned

```plaintext
Walk the attribute pipeline in src/Extension/Attributes/ from parse to render.

For every function that discards, skips, or normalizes "insignificant"
characters, output TSV: function, mechanism (PCRE character class / trim
charlist / explicit comparison), and the EXACT set of byte values it treats
as insignificant. Expand every shorthand write \x09 not "\s".

Cite file:line. Do not summarize. Do not comment on security.
```

"Expand every shorthand" is the line that matters. `\s` and `trim()` both mean "whitespace" in English, and as long as they're both spelled *whitespace* they look identical. Force the model to expand them into byte lists and they stop looking identical, because they aren't.

### Pattern 2: Asymmetry: compare the definitions, not the code

This is the core move.

```plaintext
Three layers in this pipeline independently define "whitespace":

  A = bytes PCRE's \s matches (PHP's PCRE2 build)
  B = bytes PHP's trim() strips with its DEFAULT charlist
  C = bytes the HTML5 tokenizer treats as whitespace between attributes

Produce all three sets as explicit byte lists, then produce A\B, B\A, A∩C\B.
Cite the authority for each set: PCRE docs, PHP docs, WHATWG spec.
```

Again: **a fixed answer shape.** Three sets, four derived sets, no room to trail off. And the answer, verified on this machine:

| byte | PCRE `\s` | PHP `trim()` default | HTML5 whitespace |
| --- | --- | --- | --- |
| `\x09` HT | ✅ | ✅ | ✅ |
| `\x0A` LF | ✅ | ✅ | ✅ |
| `\x0B` VT | ✅ | ✅ | ❌ |
| `\x0C` **FF** | **✅** | **❌** | **✅** |
| `\x0D` CR | ✅ | ✅ | ✅ |
| `\x00` NUL | ❌ | ✅ | ❌ |
| `\x20` SP | ✅ | ✅ | ✅ |

There it is. Out of the entire ASCII range, exactly **one** byte is matched by the regex that scans the attribute, survives the `trim()` that cleans it, and is treated as a separator by the browser that renders it.

The vulnerability is one byte wide. You can see it in a seven-row table, and you cannot see it in the code, because in the code both things are called "whitespace."

That's a two-line PHP script to confirm, and it's worth running yourself rather than trusting either me or the model:

```php
foreach (["\x09"=>"HT","\x0A"=>"LF","\x0B"=>"VT","\x0C"=>"FF","\x0D"=>"CR","\x00"=>"NUL"] as $b=>$n)
    printf("%-4s regex=%-4s trim=%s\n", $n,
        preg_match('/^\s$/',$b)===1 ? 'yes':'no',
        trim("X".$b)==="X" ? 'yes':'no');
```

### Pattern 3: Citation discipline

Every row cites `file:line`, and every *external* claim cites a spec. This matters more than usual here, because three of the seven rows above are assertions about PCRE, PHP and WHATWG rather than about the target's code exactly the kind of "everybody knows" claim a model will produce from memory and get subtly wrong. Cited claims are checkable in seconds; uncited ones cost you the afternoon you were trying to save.

### Pattern 4: The rebuttal pass

```plaintext
Argue this is NOT exploitable. Does the renderer escape attribute NAMES?
Does an HTML sanitizer downstream strip it? Does the tokenizer actually
treat U+000C as an attribute separator, or does it error-recover into
something harmless? Find the control I missed.
```

Phrase it as a command to *find the control*, not as "is this a bug?" asked neutrally a model will mostly agree with whatever you seem to want. Given a specific adversarial job, it does the job.

Two outcomes, both wins. It kills the finding for free, or it hands you the maintainer's triage reply in advance. Here it pointed at `HtmlElement::__toString()` which turned out to *confirm* the bug rather than kill it, because the renderer escapes attribute values and emits attribute names raw.

### The rules that keep it honest

1.  **The agent's conclusion is never the finding.** Its output is a lead. The finding is what survived execution.
    
2.  **Ban security adjectives in enumeration passes.** No "unsafe", no "vulnerable", until the table is built.
    
3.  **Every prompt has a knowable answer size.** Three sets. Seven rows. If you can't predict the shape, you can't detect a short answer.
    
4.  **Every phase ends in an artifact on disk** a TSV, a byte table not a narrative in a chat window.
    
5.  **Execute before reporting.** Always. See section V.
    

### What Codex will not do for you

*   **Choose the target.** Nothing in section II came from an agent. Reading a security release as a map of where the maintainer thinks the boundary is that's the work.
    
*   **Know that the docs made a promise.** The model can tell you `str_starts_with($name, 'on')` returns false. It cannot tell you the documentation says `on*` is "never allowed by default," which is what turns a parsing quirk into a High.
    
*   **Score severity.** Models inflate. `S:C` here is real (a library bug crossing into the embedding application's origin); `C:H` would not have been.
    
*   **Recognise a non-control.** It correctly listed `Xml::escape()` as escaping it doesn't notice on its own that the escaping is applied to the value and not the key.
    
*   **Disbelieve itself.** It will confirm a plausible hypothesis unless you pay for the rebuttal pass.
    

**The agent built the byte table. Knowing to ask three layers the same question was the job.**

## IV. The chain

Five links, each individually defensible:

**1: The regex consumes the byte.** The attribute-list scanner starts with `\s*`:

```php
private const SINGLE_ATTRIBUTE = '\s*([.]-?[_a-z][^\s.}]*|[#][^\s}]+|'
    . RegexHelper::PARTIAL_ATTRIBUTENAME . RegexHelper::PARTIAL_ATTRIBUTEVALUESPEC . ')\s*';
```

PCRE's `\s` matches `\x0C`, so `{​<FF>onclick="alert(1)"}` matches happily. The form feed is *inside the match*.

**2: The trim doesn't remove it.** The parser then cleans the match with PHP's `trim()` using the default charlist:

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

`trim()`'s default charlist is `" \t\n\r\0\x0B"`. **No** `\x0C`**.** The byte the regex just swallowed survives, welded to the front of the attribute name. The stored name is now the 8-byte string `"\x0Conclick"`.

**3: The filter compares against a literal.** The `on*` blocklist added in 2.7.0:

```php
// No allowlist?
if ($allowList === []) {
    // Just remove JS event handlers
    if (\str_starts_with($attrNameLower, 'on')) {
        unset($attributes[$name]);
    }
    continue;
}
```

`"\x0Conclick"` does not start with `"on"`. The filter returns false and the attribute is kept. Note the branch: this is the path taken when **no allowlist is configured** the default.

**4: The renderer emits names unescaped.** `HtmlElement::__toString()`:

```php
$result .= ' ' . $key . '="' . Xml::escape($value) . '"';
```

The **value** goes through `Xml::escape()`. The **key** is concatenated raw, on the reasonable assumption that upstream filtering only ever hands it well-formed names.

**5: The browser disagrees with PHP about where the name begins.** The output is `<p \x0Conclick="alert(1)">`. Per the WHATWG spec, U+000C is ASCII whitespace, so the tokenizer reads *space, form feed* as one whitespace run and the attribute name as `onclick`. A genuine event handler.

### The core reasoning

> **Three parsers in one pipeline each define "whitespace," and no two definitions agree.**
> 
> *   PCRE's `\s` includes FF and excludes NUL.
>     
> *   PHP's `trim()` default includes NUL and VT, and excludes FF.
>     
> *   The HTML5 tokenizer includes FF and excludes VT and NUL.
>     
> 
> The exploitable byte is the one in PCRE's set **and** the browser's set but **not** PHP's. There is exactly one.

Call it a **definition mismatch** bug. It's the same family as validate-before-canonicalize the guard is real and it runs on a slightly different string than the one that reaches the sink except here the divergence isn't path normalization, it's two libraries' opinions about what counts as a space. The sanitizer and the browser disagree about *where the attribute name begins*.

And there's a second, sharper observation sitting in that same function. `filterAttributes()` has two branches:

*   **No allowlist configured** → blocklist: drop names starting with `on`.
    
*   **Allowlist configured** → allowlist: `isset($allowList[$name])`, drop everything else.
    

The allowlist branch is immune. `"\x0Conclick"` isn't a key in the allowlist, so it's dropped no string manipulation involved, nothing to obfuscate past. Confirmed on the vulnerable release:

```plaintext
default (empty allow list)   <p <FF>onclick="alert(1)">hello</p>
explicit allow list          <p>hello</p>
```

**The secure branch required opting in. The default took the bypassable one.** That's the finding behind the finding, and it's the part worth carrying to your own code: a blocklist and an allowlist in the same function are not two implementations of one policy, they're two different security postures, and shipping the weaker one as the default is a decision.

## V. Proof

An agent's table is a lead. This is the part that makes it a report and it's why I install the real published artifact rather than reasoning about the diff.

Unmodified `league/commonmark` from Packagist, default-secure configuration (`html_input: escape`, `allow_unsafe_links: false`), plain `AttributesExtension`:

**On 2.9.0 the release from six days earlier:**

```plaintext
plain onclick (control)        filtered
                               <p>hello</p>

FF + onclick                   *** HANDLER REACHED OUTPUT ***
                               <p <FF>onclick="alert(1)">hello</p>

FF + onerror on image          *** HANDLER REACHED OUTPUT ***
                               <p><img <FF>onerror="alert(1)" src="https://example.com/x.png" alt="x" /></p>

FF + href javascript:          *** URI REACHED OUTPUT ***
                               <p><a <FF>href="javascript:alert(1)" href="https://example.com">click</a></p>

href + FF before =             *** URI REACHED OUTPUT ***
                               <p><a href<FF>="javascript:alert(1)" href="https://example.com">click</a></p>

VT + onclick (should be safe)  filtered
                               <p>hello</p>
```

**On 2.9.1 every row** `filtered`**.**

Four properties of that run are deliberate, and I build every PoC this way:

*   **The control is in the same run.** Plain `{onclick=...}` being filtered proves the 2.7.0 protection is present and working, so "your build didn't have the filter" is off the table before anyone asks.
    
*   **The negative control is in the same run.** `\x0B` (vertical tab) is *also* whitespace to PCRE and *also* survives nothing it's stripped by `trim()`, so it's harmless. Including a byte that *doesn't* work proves the mechanism is the specific set difference and not "control characters generally." It also guards the fix: a maintainer who patches by adding only `\x0C` to a list has a test that still passes.
    
*   **The success condition is the emitted string, not an exit code.** `onclick` appearing in the output is the bug. HTTP 200 would prove nothing.
    
*   **It's two** `composer require`**s and one script.** A maintainer reproduces it in ninety seconds. That is the single highest-leverage thing you can put in a report.
    

Two of those rows deserve a note beyond the `on*` filter:

**The** `href` **rows defeat** `allow_unsafe_links` **too** a *separate* protection, and one that had been hardened six days earlier. Look at the emitted order: `<a \x0Chref="javascript:alert(1)" href="https://example.com">`. The injected attribute is emitted **first**, and per the HTML5 spec a duplicate attribute keeps the *first* occurrence. So the browser reads the element's `href` as `javascript:alert(1)` and discards the legitimate one. The link the user clicks is the attacker's.

**The image row needs no interaction at all.** `{<FF>onerror="..."}` on an image that fails to load fires on render. That's `UI:N`, and it's the reason this scores 7.2 rather than something with a `UI:R` discount.

### Impact

Any application rendering untrusted Markdown with `AttributesExtension` enabled and default configuration gets **stored XSS** the payload persists wherever the Markdown does. Session theft, actions as the victim, account compromise. `S:C` because the flaw is in a library and the impact lands in the embedding application's origin.

I scored it `C:L/I:L`, not `C:H/I:H`. XSS severity depends entirely on what the host application keeps in that origin, and a library advisory can't know. Claiming High confidentiality on someone else's application is the kind of overreach that gets an otherwise airtight report re-litigated.

## VI. The fix

`dfcdf455`, released in 2.9.1 the next day. It's a two-layer fix, and the second layer is the one that matters.

**Layer 1: make the parser agree with itself.** A named charlist, used at every trim in the parse path:

```php
/**
 * PCRE's `\s` matches the form feed that PHP's default trim charlist omits, so the
 * separators SINGLE_ATTRIBUTE accepts must be trimmed with this list instead - otherwise
 * that byte survives inside an attribute name, where a browser reads it as a separator.
 */
private const WHITESPACE = " \t\n\r\0\x0B\x0C";
```

That closes the reported input. On its own it would be a narrow fix it fixes `\x0C` because `\x0C` is what was reported.

**Layer 2: stop enumerating badness.** In `filterAttributes()`, a positive well-formedness gate in front of every other check:

```php
// The checks below compare against literal names, and the renderer emits names
// without escaping them, so anything that isn't a well-formed attribute name
// would slip past both
if (\preg_match('/^' . RegexHelper::PARTIAL_ATTRIBUTENAME . '$/i', (string) $name) !== 1) {
    unset($attributes[$name]);
    continue;
}
```

`PARTIAL_ATTRIBUTENAME` is `[a-z_:][a-z0-9:._-]*`. Anything that isn't that shape is dropped before the `on` check, before the unsafe-link check, regardless of what byte was used or which layer's definition of whitespace it exploited. **This is the fix that closes the class rather than the input**, and it's the one I'd point at in a code review: the blocklist branch stops being the weak branch, because nothing malformed reaches it.

The comment above it is also doing real work. It records *why* the gate exists that the downstream checks compare literals and the renderer emits names raw so the next person to touch this function knows what breaks if they remove it. Most security fixes don't leave that behind.

**The regression test is the third thing worth stealing.** `ObfuscatedAttributeNameTest` covers six cases: the `on*` bypass in inline and block syntax, the auto-firing `onerror` image, both `href` positions, and critically `\x0B` as a case that must *still* be filtered. That last one is an anti-narrowing test: it fails if someone "simplifies" the fix into a special case for one byte.

**What didn't need to ship:** nothing I'd push back on. The report suggested the charlist fix; the maintainer added the well-formedness gate on top, which is strictly better than what was proposed. That's the good outcome you hand over a root cause and a maintainer who knows the codebase generalizes it further than you could.

## VII. Timeline

| Date | Event |
| --- | --- |
| 2025-05-05 | 2.7.0 ships "Fix XSS in AttributesExtension" introduces `filterAttributes()` and the `on*` blocklist. Also introduces the affected range. |
| 2026-08-03 | 2.9.0 ships as a security release: 5 DoS + GHSA-29pj-957v-52mc, control-character obfuscation in attribute **values**. |
| 2026-08-03 → 08-08 | Variant hunt off that fix. Byte-set comparison across PCRE / PHP `trim()` / HTML5 finds a one-byte gap. Reproduced on stock 2.9.0; rebuttal pass run; reported privately. |
| 2026-08-08 | Fix committed `dfcdf455`, charlist plus well-formedness gate, six regression cases. |
| 2026-08-09 | Released as **2.9.1**. |
| 2026-08-09 | **GHSA-f8fg-pg57-v4j8** published, High, CVSS 7.2, credited to StarPlatinu. |

Six days from someone else's security release to an advisory on the variant it didn't cover.

## VIII. What generalizes

1.  **Subscribe to security releases and read every fix as a map.** A shipped fix tells you the class exists, the maintainer accepts it, and exactly how far they looked. The gap between the reported input and the class is the next finding. This is the cheapest signal in open-source security research and almost nobody works it.
    
2.  **Run agreement audits.** Wherever two layers independently define the same concept, the disagreement set is the attack surface. Whitespace is one. Others worth the same table: what counts as a path separator, case-folding rules, Unicode normalization forms, "is this a valid URL," integer parsing, and every place a language's stdlib and a spec both define "trim."
    
3.  **Make the model expand shorthand into values.** `\s` and `trim()` both read as "whitespace" and are not the same set. Two abstractions with the same name look identical until you force both into explicit byte lists and that expansion is exactly the tedium an agent is for.
    
4.  **Include a negative control in the PoC.** A byte that *doesn't* work proves you understand the mechanism, and it survives into the maintainer's test suite as an anti-narrowing guard.
    
5.  **Blocklist and allowlist in one function are two security postures.** Check which one is the default. Here, the opt-in path was immune and the default path was bypassable.
    

And the one that isn't about tooling: **score honestly and say why.** `C:L/I:L` on a library XSS, not `C:H`. A finding with one obviously-inflated component invites the maintainer to re-litigate all of them.