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.
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.onclickoronerror) 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
CWE: CWE-79 (XSS), CWE-86 (Improper Neutralization of Invalid Characters in Identifiers)
Merged fix:
dfcdf455released 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 theallow_unsafe_linksprotection to be bypassed viahrefandsrcattributes (GHSA-29pj-957v-52mc)
Read that as an attacker and it says four things at once:
The class is confirmed present. Control-character obfuscation defeats this library's filters. Not a theory the maintainer just shipped a fix saying so.
The maintainer accepts and ships this class, fast, with credit. That's the difference between a finding and a wasted week.
The fix has a stated scope, and the scope is narrow: attribute values, specifically
hrefandsrc, specifically scheme detection.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
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.
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:
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
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
The agent's conclusion is never the finding. Its output is a lead. The finding is what survived execution.
Ban security adjectives in enumeration passes. No "unsafe", no "vulnerable", until the table is built.
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.
Every phase ends in an artifact on disk a TSV, a byte table not a narrative in a chat window.
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 sayson*is "never allowed by default," which is what turns a parsing quirk into a High.Score severity. Models inflate.
S:Chere is real (a library bug crossing into the embedding application's origin);C:Hwould 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*:
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:
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:
// 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():
$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
\sincludes 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:
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:
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 bytrim(), 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\x0Cto a list has a test that still passes.The success condition is the emitted string, not an exit code.
onclickappearing in the output is the bug. HTTP 200 would prove nothing.It's two
composer requires 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:
/**
* 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:
// 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
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.
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."
Make the model expand shorthand into values.
\sandtrim()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.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.
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.





