fix(security): 2 improvements across 2 files#1859
Conversation
- Security: Regular expression injection / ReDoS risk from unescaped search input - Security: Regex construction from unescaped characters in ObjectKey renderer Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
- Security: Regular expression injection / ReDoS risk from unescaped search input - Security: Regex construction from unescaped characters in ObjectKey renderer Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
|
@tomaioo: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
jerelmiller
left a comment
jerelmiller
left a comment
There was a problem hiding this comment.
I'd love to use RegExp.escape, otherwise this looks good.
Before I accept this, can you sign the CLA please?
| ? new RegExp( | ||
| `(${softWrapCharacters | ||
| .map((character) => | ||
| character.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") |
There was a problem hiding this comment.
I'd love to use RegExp.escape if we could. That supports Chrome and Firefox versions that have been out for a year so I think its pretty safe to use it at this point in the extension. Can you update these to RegExp.escape?
| character.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") | |
| RegExp.escape(character) |
| return <>{value}</>; | ||
| } | ||
|
|
||
| const escapedSearchTerm = searchTerm.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); |
There was a problem hiding this comment.
| const escapedSearchTerm = searchTerm.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); | |
| const escapedSearchTerm = RegExp.escape(searchTerm); |
#2025 Bundle Size — 2.01MiB (~+0.01%).c10b16d(current) vs b8b6f7c main#2022(baseline) Warning Bundle contains 16 duplicate packages – View duplicate packages Bundle metrics
|
| Current #2025 |
Baseline #2022 |
|
|---|---|---|
 Initial JS |
1.73MiB(+0.01%) |
1.73MiB |
 Initial CSS |
0B |
0B |
| Cache Invalidation |
78.88% |
78.88% |
| Chunks |
5 |
5 |
| Assets |
237 |
237 |
| Modules |
1529 |
1529 |
| Duplicate Modules |
134 |
134 |
| Duplicate Code |
5.69% |
5.69% |
| Packages |
180 |
180 |
| Duplicate Packages |
12 |
12 |
Bundle size by type 
1 change 1 regression
| Current #2025 |
Baseline #2022 |
|
|---|---|---|
| JS |
1.73MiB (+0.01%) |
1.73MiB |
| Other |
251.83KiB |
251.83KiB |
| IMG |
35.85KiB |
35.85KiB |
| HTML |
857B |
857B |
Bundle analysis report Branch tomaioo:fix/security/regular-exp... Project dashboard
Generated by RelativeCI Documentation Report issue
commit: |
…s://developer.mozilla. Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
const escapedSearchTerm = RegExp.escape(sear Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
3 participants
Summary
fix(security): 2 improvements across 2 files
Problem
Severity:
Medium| File:src/application/components/HighlightMatch.tsx:L7HighlightMatchbuilds aRegExpdirectly fromsearchTerm(new RegExp(searchTerm, "i")). IfsearchTermis user-controlled, attackers can supply regex metacharacters to trigger expensive backtracking (ReDoS) or runtime exceptions (invalid regex), causing UI freezes or crashes.Solution
Escape user input before creating a regex (e.g.,
searchTerm.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")), enforce max input length, and wrap regex creation intry/catchto prevent crashes.Changes
src/application/components/HighlightMatch.tsx(modified)src/application/components/ObjectViewer/ObjectKey.tsx(modified)