Skip to content

fix: stop the CSV detection hang, clear CodeQL alerts, fix the mobile output bar - #4

Merged
m4sh-wacker merged 2 commits into
mainfrom
fix/hang-codeql-mobile
Oct 19, 2026
Merged

m4sh-wacker merged 2 commits into
mainfrom
fix/hang-codeql-mobile

Conversation

@m4sh-wacker

Copy link
Copy Markdown
Collaborator

Why

A full check of the live site and the repository turned up one serious bug and two things that looked bad now that the project is under OWASP.

1. CSV detection could freeze the page (CodeQL js/redos ×2)

The CSV detection pattern, [^\n]*,[^\n]* per line, could split a line at any of its commas. On input that failed at the end, those choices multiplied line by line: 16 lines of ,,, froze the page on the live site, and 18 took over 30 seconds in Node. Detection runs on whatever is pasted, so a 70-character string was enough.

Each line now matches in exactly one way:

/^[^\r\n,]*,[^\r\n]*(?:\r?\n[^\r\n,]*,[^\r\n]*)+\r?$/
  • Same answer as the old pattern on 18 hand-picked cases, ordinary CSV and CRLF included.
  • 20,000 hostile lines: 2 ms. A 50,000-row CSV is still detected in 3 ms.
  • detection.redos.test.ts pins it. Against the old pattern the test hangs; against the new one it passes.

2. CodeQL alerts

Alert Count Change
js/insecure-randomness 13 Step, layer and recipe ids come from one helper, randomId, built on crypto.getRandomValues. They were never secret, but no identifier is drawn from Math.random now. getRandomValues (unlike randomUUID) also works in the single-file build opened from disk.
js/incomplete-multi-character-sanitization 4 Strip HTML tags and the HTML/XML comment strippers repeat until nothing changes (removeAll), so a removal cannot rebuild a tag or comment. Covered by strip.test.ts.
js/redos 2 See above.
js/incomplete-sanitization (report table) 1 No code change. In a GFM table, | is the escape cells honour and backslashes inside code spans are literal, so escaping \ too would print Windows paths as C:\Users. False positive.
js/missing-origin-check (engine worker) 1 No code change. It is a dedicated worker: only the page that created it can post to it, and MessageEvent.origin is empty there. False positive.

3. Mobile output bar

On a phone, "147 bytes · 7 lines" and the AUTO-DECODED badge wrapped and spilled over the header. The pane bar now stays on one line (badges clip instead of wrapping), and the output size is left to the status bar on narrow screens.

Checked locally

  • npm run lint, npm run typecheck
  • npm run test -- --run: 63 files, 1117 tests (1109 existing + 8 new)
  • npm run build: no deprecation warnings
  • In Chrome on the built site: Base64 → gzip → JSON, JWT, double Base64 and hex still decode; 16, 20 and 200 hostile lines leave the page responsive and the next input decodes; Strip HTML tags leaves no tag; the mobile output bar stays on one line; no console errors.

… output bar

- CSV detection: the pattern could split each line at any of its commas,
  so on input that failed at the end the choices multiplied line by line.
  Sixteen lines of ",,," froze the page, and detection runs on whatever is
  pasted. Each line now matches in exactly one way. A regression test pins
  it, and it agrees with the old pattern on ordinary CSV.
- Step, layer and recipe ids come from crypto.getRandomValues through one
  helper, randomId, instead of Math.random in thirteen places. They were
  never secret, but nothing that looks like an identifier is drawn from
  Math.random now. getRandomValues also works in the single-file build.
- Strip HTML tags and the HTML/XML comment strippers repeat until nothing
  changes, so a removal can no longer rebuild a tag or comment out of the
  pieces around it.
- The pane bar stays on one line: on a narrow screen its badges are clipped
  instead of wrapping over the header, and the output size is left to the
  status bar on phones.
Comment thread src/engine/strip.test.ts Fixed
CodeQL reads any script-tag regex as a tag filter, even in a test, and
flagged the example. A plain pattern shows the same thing: one pass over
"aabb" removing "ab" leaves a new "ab" behind.
@m4sh-wacker
m4sh-wacker enabled auto-merge October 19, 2026 14:49
@m4sh-wacker
m4sh-wacker merged commit 35ba77c into main Oct 19, 2026
6 checks passed
@m4sh-wacker
m4sh-wacker deleted the fix/hang-codeql-mobile branch October 19, 2026 14:51
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