Skip to content

security: linear trailing-newline trim and complete escaping (redos + incomplete-sanitization) - #78

Merged
alloevil merged 3 commits into
masterfrom
security/regex-escaping
Oct 18, 2026
Merged

alloevil merged 3 commits into
masterfrom
security/regex-escaping

Conversation

@alloevil

Copy link
Copy Markdown
Owner

Fixes the remaining 2 js/polynomial-redos and 2 js/incomplete-sanitization CodeQL alerts (follow-up to #77).

Triage

Alert Location Verdict / fix
js/polynomial-redos ×2 lib/library.js content.replace(/\n*$/, '\n') in serializeLibraryFile / serializeInstalledFile (content comes from the library API body) Real (polynomial backtracking on long newline runs). Replaced with a linear scan withOneTrailingNewline(); output identical.
js/incomplete-sanitization lib/export.js markdown metadata cell .replace(/|/g, '\|') Real: a value ending in \ (e.g. a Windows cwd) turned | back into a live column separator. Now escapes backslashes first, then pipes (escapeTableCell).
js/incomplete-sanitization public/js/app.js CSS.escape fallback id.replace(/"/g, '\\"') Real (only hit when CSS.escape is missing): backslashes weren't escaped. Now id.replace(/["\\]/g, '\\$&'). Legacy UI is frozen except security fixes; this is one.

No dismissals.

Tests (existing files; file count stays 22)

  • export.test.js: cwd C:\proj|x\ renders C:\\proj\|x\\, every pipe preceded by an odd backslash run.
  • unit.test.js: trailing-newline collapse cases + 400k-newline input finishes < 1 s.
  • 356 → 358 tests; claims.json + ROADMAP updated (gate unchanged).

Local evidence (Node 22.23.3)

…lynomial-redos, 2 js/incomplete-sanitization)

- lib/library.js: replace content.replace(/\n*$/, '\n') with a linear scan
  (withOneTrailingNewline); same output, no polynomial backtracking.
- lib/export.js: markdown table cells escape backslashes before pipes.
- public/js/app.js: CSS.escape fallback escapes backslashes as well as quotes.
Tests: 2 new cases in existing files.
@alloevil
alloevil merged commit f409ae9 into master Oct 18, 2026
5 checks passed
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.

1 participant