Skip to content

fix: CodeQL alerts (bounded regexes, null-prototype export groups, exact site URL match) - #232

Merged
dgesteves merged 4 commits into
mainfrom
fix/codeql-onsystem
Oct 23, 2026
Merged

dgesteves merged 4 commits into
mainfrom
fix/codeql-onsystem

Conversation

@dgesteves

@dgesteves dgesteves commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

Fixes the open CodeQL alerts in the token readers, the token export and the site's docs links. Each regex matches the same strings as before, with the same capture groups. The only difference is that a failing input now has one way to match, not thousands.

Alerts

The numbers are code scanning alert numbers (Security > Code scanning), not issues or pull requests.

ReDoS. Times are for the attack input, before and after. Before means the old regex alone, measured at 10,000 characters where 100,000 did not finish; the growth is quadratic.

The export test's ten 100,000-character tokens plus the 126-character one go through toDtcg and toDesignMd in 34 ms, measured under coverage instrumentation. Before the fix, a single such token took tens of seconds.

Prototype pollution:

URL check:

  • feat(site): add the website, with the check_ui loop, the benchmark and how it works #23 js/incomplete-url-substring-sanitization in site/lib/site.ts: resolveDocHref makes a link a site path only when it is exactly https://onsystem.vercel.app or starts with https://onsystem.vercel.app/. A look-alike host such as https://onsystem.vercel.app.example.com/x stays as written; before, it became the broken path .example.com/x. Every docs link today starts with the site URL and /, so the site's output does not change.

Not in this PR: #3 js/incomplete-sanitization in action/check.mjs was triaged as a false positive and is handled separately.

Equivalence

The old and new regexes were compared on 400,000 random strings and 400,000 grammar-generated strings (HSL and RGB channels, composites). Matches and capture groups showed 0 differences. For var() the comparison covered the index, length, name and trimmed fallback.

The new tests check that the old valid forms read the same way, in both the DTCG and the DESIGN.md output. They include 1., .5rem, 1.5EM, -30deg, 50%, .5% / .5, 22,163,74 / 0.5, var( --missing , 0.5rem ) and composites with spaces around :.

Tests

  • test/tokens.test.ts: lengths, var() fallbacks and bare channels read as before, and each 100,000-character attack input finishes in under 100 ms.
  • test/export.test.ts: numbers, lengths, composites and channels export as before. The long tokens export in under 200 ms. A __proto__ token stays in its group, and Object.prototype gets no polluted property.
  • site/test/markdown.test.ts: resolveDocHref on the site URL, a path under it and a look-alike host.

Patch changeset: .changeset/long-token-values.md.

Checks run

  • pnpm lint, pnpm format:check and pnpm typecheck
  • pnpm test: 1181 passed
  • The new tests under --coverage
  • pnpm build and pnpm --filter site generate
  • Site test (274 passed), typecheck and lint

…TCG groups

The patterns that read a number (lengths, line heights, DESIGN.md
dimensions, bare HSL and RGB channels) allowed a run of digits to be
split between `\d+` and `\d*` in every way, so a failing match on a long
value took quadratic time: 25 s for 100,000 digits. They now read the
same strings with one way to match. The composite typography pattern and
the var() fallback dropped a `\s*` that overlapped the part after it.

toDtcg builds its groups with Object.create(null), so a token path with
a `__proto__` part is an ordinary key and cannot reach Object.prototype.

The site's resolveDocHref turns only the site's own URL, or a path under
it, into a site path, not a host that merely starts with it.
Comment thread src/tokens/units.ts Fixed
… stay linear

CodeQL's new alert on this PR: with `[^)]+`, each `var(` in an unclosed
run such as `var(---,var(---,(...` scanned to the end of the value for its
`)`. 20,000 repeats took 7.4 s; with `[^()]+` they take 0.4 ms. A fallback
that holds parentheses had no px value before either, and still has none.
The previous commit's `[^()]+` fallback kept unclosed runs linear but lost
values main read inside calc(): calc(2 * var(--a, var(--b))) exported as
16px on main and as the raw string with it. expandVars now scans each
var() to its matching parenthesis, so a fallback may hold calc() or
another var(), and an unclosed var() ends the scan, so no later `var(` is
scanned to the end again. A differential fuzz against a CSS-correct
reference (300,009 expressions) finds no value it gets wrong, none main
got right that it loses, 54,619 it now reads that main did not, and
9,705 main read wrong that it now reads right. The attack input
(`var(---,` + `var(---,(` x 20,000) takes 0.5 ms.
@dgesteves
dgesteves merged commit f286e9d into main Oct 23, 2026
19 checks passed
@dgesteves
dgesteves deleted the fix/codeql-onsystem branch October 23, 2026 21:33
@dgesteves-release dgesteves-release Bot mentioned this pull request Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants