Skip to content

fix(rrweb-snapshot): rewrite vulnerable regexes flagged by CodeQL#289

Merged
logaretm merged 2 commits intosentry-v2from
awad/sdk-1099-codeql-regex-fixes
Apr 28, 2026
Merged

fix(rrweb-snapshot): rewrite vulnerable regexes flagged by CodeQL#289
logaretm merged 2 commits intosentry-v2from
awad/sdk-1099-codeql-regex-fixes

Conversation

@logaretm
Copy link
Copy Markdown
Member

@logaretm logaretm commented Apr 27, 2026

Rewrites two regexes flagged by CodeQL: the CSS-comment-stripping regex in css.ts (CodeQL alerts 5 and 6) had nested quantifiers causing exponential backtracking on unterminated comments, and the whitespace-trim regex in snapshot.ts (CodeQL alert 4) was polynomial.

Both replacements are observably identical for well-formed input; the new comment regex additionally strips two malformed-but-valid comment shapes (/***/, /**foo**/) that the old one left unchanged.

Copilot AI review requested due to automatic review settings April 27, 2026 18:59
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Apr 27, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens rrweb-snapshot against CodeQL-flagged regex performance issues by rewriting a selector comment-stripping regex to avoid catastrophic backtracking and simplifying a whitespace-only text-node check using native trim().

Changes:

  • Rewrites selector comment stripping in the CSS parser to a linear-time lazy match to prevent ReDoS scenarios.
  • Replaces a multiline whitespace-trimming regex with .trim() for whitespace-only text node detection.
  • Adds unit tests covering selector comment stripping behavior and a regression guard for unterminated comment inputs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/rrweb-snapshot/src/css.ts Replaces the vulnerable selector comment-stripping regex with a linear-time alternative.
packages/rrweb-snapshot/src/snapshot.ts Uses native .trim() for whitespace-only text-node detection (replacing regex-based trimming).
packages/rrweb-snapshot/test/css.test.ts Adds tests for selector comment stripping and a regression test to prevent catastrophic backtracking.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@logaretm logaretm requested review from billyvg and chargome April 27, 2026 19:04
logaretm and others added 2 commits April 28, 2026 10:15
Three regex issues:

1. css.ts:445 (CodeQL #5, #6) — the comment-stripping regex used in
   selector parsing had nested `*` quantifiers over alternatives that
   could match the same characters, causing exponential backtracking
   on inputs like `/*` followed by many `\n*` with no closing `*/`.
   Rewritten as a lazy match (`/\/\*[\s\S]*?\*\/+/g`), which is
   linear-time and produces identical output for all well-formed
   comments. Adds a regression test that hangs the old regex.

2. snapshot.ts:1348 (CodeQL #4) — flagged for polynomial-time
   whitespace trimming via `replace(/^\s+|\s+$/gm, '')`. Replaced
   with `trim()`, which has the same observable behavior for the
   length-zero check the call site does and is implemented natively.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@logaretm logaretm force-pushed the awad/sdk-1099-codeql-regex-fixes branch from d07cd5d to 9ded3a2 Compare April 28, 2026 14:15
@logaretm logaretm merged commit 7dcdae0 into sentry-v2 Apr 28, 2026
22 of 23 checks passed
@logaretm logaretm deleted the awad/sdk-1099-codeql-regex-fixes branch April 28, 2026 15:53
logaretm added a commit to getsentry/sentry-javascript that referenced this pull request May 8, 2026
Bumps all rrweb dependencies in tests and internal packages to the
latest versions.

The changelog for these new releases is:

- [(rrweb-snapshot) Rewrite vulnerable
regexes](getsentry/rrweb#289)
- [Use CSS Declaration replaceSync to parse styles to avoid CSP
violations](getsentry/rrweb#286)
- [Wrap iframe contentWindow access in
try-catch](getsentry/rrweb#275)


Full changelog can be [found
here](https://github.com/getsentry/rrweb/releases/tag/2.42.0).
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.

3 participants