fix(react): remove ReDoS paths in data URL and toolbar test-id parsing#962
fix(react): remove ReDoS paths in data URL and toolbar test-id parsing#962jedrazb wants to merge 1 commit into
Conversation
Parse the data URL MIME type with index math instead of a regex that
retried at every ':' and scanned to the end of the string, and bound
the toolbar test-id parenthesis strip to '[^()]' so it cannot backtrack
across unmatched '('. Both ran in quadratic time on crafted input.
Resolves CodeQL js/polynomial-redos in InsertImageDialog.tsx and
Toolbar.tsx.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ Posted by the CLA bot. |
Greptile SummaryThis PR closes two CodeQL
Confidence Score: 4/5Safe to merge; both fixes are narrow, correct, and tested. No layout, paint, or PM-state paths are touched. The MIME extraction logic is correct for all standard data URL shapes and demonstrably linear. The
|
| Filename | Overview |
|---|---|
| packages/react/src/components/dialogs/InsertImageDialog.tsx | Replaces the backtracking /:(.*?);/ regex with indexOf-based MIME extraction; logic is correct for all standard data URL shapes, though the semi === -1 branch admits an unbounded MIME candidate from adversarial input. |
| packages/react/src/components/Toolbar.tsx | Single-character class fix from [^)] to [^()] correctly prevents quadratic backtracking on parenthesis-heavy titles; behavior for well-formed input is identical. |
| packages/react/src/components/dialogs/dataUrlToBlob.test.ts | New test file covering MIME parsing, empty-MIME fallback, and a ReDoS timing guard; timing threshold of 1 000 ms is workable but generous for what should be a microsecond-scale operation. |
| .changeset/react-redos-hardening.md | Correct patch-level changeset for @eigenpal/docx-editor-react; consumer-facing description is clear and concise. |
Reviews (1): Last reviewed commit: "fix(react): remove ReDoS paths in data U..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| test('stays linear on an adversarial prefix (no ReDoS)', () => { | ||
| // A long `:a:a:a…` prefix with no `;` used to backtrack quadratically. | ||
| const evil = ':' + ':a'.repeat(200_000) + ',QUJD'; | ||
| const start = performance.now(); | ||
| dataUrlToBlob(evil); | ||
| expect(performance.now() - start).toBeLessThan(1_000); | ||
| }); |
There was a problem hiding this comment.
The 1 000 ms threshold is generous in isolation, but on a heavily loaded CI runner a GC pause or context switch during the atob/Blob path could push runtime above 1 s even for an O(n) implementation. It's also worth noting that the adversarial string has no semicolon, so mime is set to the entire 400 000-char candidate before Blob construction — the test proves the implementation is linear, but only indirectly. Consider asserting a tighter bound (e.g. 200 ms) or adding a sub-millisecond relative comparison to make the intent harder to miss.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // `[^()]` (not `[^)]`) so the run can't span unmatched `(` and | ||
| // backtrack quadratically on a long parenthesis-heavy title. | ||
| .replace(/\([^()]*\)/g, '') | ||
| .trim(); |
There was a problem hiding this comment.
The dataUrlToBlob fix ships with a ReDoS timing guard and functional tests, but the [^()]* change has no corresponding test. A simple unit test that passes a parenthesis-heavy title string and asserts the expected test-id output (and optionally a timing guard for adversarial input like '('.repeat(50_000)) would close the coverage gap and make a future regression visible at the unit level rather than only through browser E2E tests.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const candidate = header.slice(colon + 1, semi === -1 ? undefined : semi); | ||
| if (candidate) mime = candidate; |
There was a problem hiding this comment.
semi === -1), header.slice(colon + 1, undefined) returns the entire tail of parts[0] after the colon. For adversarial input with no semicolon and a very long colon-prefixed string, this silently sets a megabyte-scale string as the Blob type. Adding a basic sanity check — e.g. requiring a / in the candidate (all real MIME types have one) — would prevent a huge string from leaking into the Blob and make the fallback behavior more predictable for malformed input.
| const candidate = header.slice(colon + 1, semi === -1 ? undefined : semi); | |
| if (candidate) mime = candidate; | |
| const candidate = header.slice(colon + 1, semi === -1 ? undefined : semi); | |
| if (candidate && candidate.includes('/')) mime = candidate; |
1 participant
Closes two high-severity CodeQL js/polynomial-redos alerts in the React adapter. Both regexes had many candidate start positions, each scanning to the end of the string, so a long crafted input ran in quadratic time.
InsertImageDialog.tsxdataUrlToBlob—/:(.*?);/is replaced byindexOf-based MIME extraction (no regex, linear, and slightly more accurate for types without a;base64marker).Toolbar.tsxtest-id helper —/\([^)]*\)/gbecomes/\([^()]*\)/g, so the run cannot span unmatched(and backtrack.No Vue counterpart exists for either path (the Vue adapter has no
dataUrlToBloband uses static test ids), so this is correctly React-only.Added
dataUrlToBlob.test.tscovering MIME parsing, theimage/pngfallback, and a ReDoS guard that must finish near-instantly on adversarial input (it previously took ~37s).🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.