fix(security): resolve js/polynomial-redos in _.trim (CodeQL #161) - #627
Closed
scotmatson wants to merge 1 commit into
Closed
fix(security): resolve js/polynomial-redos in _.trim (CodeQL #161)#627scotmatson wants to merge 1 commit into
scotmatson wants to merge 1 commit into
Conversation
Replace the MDN trim polyfill's anchored-alternation regex (/^[\s\xA0]+|[\s\xA0]+$/g) with native String.prototype.trim. CodeQL flagged the regex as polynomial ReDoS: on attacker-influenceable DOM text (autocapture reads element textContent through _.trim) a long run of '\t' followed by a non-space forces O(n^2) backtracking. Native trim strips the identical set (ECMAScript WhiteSpace + LineTerminator, including NBSP \xA0 and BOM ) in linear time, so behavior is unchanged. The codebase already relies on native .trim() in autocapture/utils.js and recorder/session-recording.js, so this adds no new browser-support assumption. Adds a _.trim unit suite covering whitespace handling, \xA0/ equivalence to the former regex, and a linear-time guard against regression. Resolves CodeQL alert #161 (js/polynomial-redos). The dist/ alert clears at the next release rebuild, since dist is regenerated from src at release time. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Moving this to the private repo at Jakub's request — superseded by mixpanel/mixpanel-js-private#482 (identical diff; private and public master are the same commit). Closing to avoid duplicate review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
CodeQL alert #161 (
js/polynomial-redos, high) flags the_.trimpolyfill insrc/utils.js:The anchored-alternation regex backtracks in O(n²) on a long run of
\tfollowed by a non-space. This is reachable from attacker-influenceable input: autocapture passes elementtextContentthrough_.trim(src/autocapture/utils.js).Change
Replace it with native
String.prototype.trim().trim()strips exactly the ECMAScript WhiteSpace + LineTerminator set, which includes NBSP (\xA0) and BOM () — the same characters the old regex targeted — in linear time..trim()insrc/autocapture/utils.jsandsrc/recorder/session-recording.js.Testing
npm run lint— cleannpm run unit-test— 686 passing, 0 failing_.trimunit suite (5 tests): whitespace handling, interior preservation,\xA0/equivalence to the former regex, all-whitespace input, and a linear-time guard (200k\t) that fails if a super-linear implementation ever regresses.Impact
Resolves CodeQL #161. Since
dist/is rebuilt fromsrc/at release time, the correspondingdist/*alert clears on the next release rebuild.🤖 Generated with Claude Code