Skip to content

Fix the second denial-of-service vulnerability in parsePatch#649

Merged
ExplodingCabbage merged 2 commits intomasterfrom
fix-redos-2
Jan 7, 2026
Merged

Fix the second denial-of-service vulnerability in parsePatch#649
ExplodingCabbage merged 2 commits intomasterfrom
fix-redos-2

Conversation

@ExplodingCabbage
Copy link
Collaborator

@ExplodingCabbage ExplodingCabbage commented Jan 7, 2026

This fixes the second, uh, ReDOS reported in #644. Though there's worse here than just a ReDOS!

First, I will explain the ReDOS. The regex /^(---|\+\+\+)\s+(.*)\r?$/ takes n^2 time to execute against a string that ends with n spaces followed by a \u2028 and a non-space character, for similar reasons to those explained in #647. The change here fixes that.

But it was worse than that! The bigger problem is that the code around this regex used to go into an infinite loop (consuming more and more memory until an OOM error occurs) when called with even small adversarial inputs. e.g.:

> const { parsePatch } = require('./libcjs');
undefined
> parsePatch('--- x\u2028x')

<--- Last few GCs --->

[87253:0x45b0d000]    37408 ms: Mark-Compact 4051.2 (4131.1) -> 4042.3 (4138.1) MB, pooled: 0 MB, 972.26 / 0.00 ms  (average mu = 0.160, current mu = 0.127) allocation failure; scavenge might not succeed
[87253:0x45b0d000]    38452 ms: Mark-Compact 4058.0 (4138.1) -> 4046.7 (4142.6) MB, pooled: 0 MB, 1037.85 / 0.00 ms  (average mu = 0.114, current mu = 0.006) allocation failure; scavenge might not succeed


<--- JS stacktrace --->

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
----- Native stack trace -----

 1: 0xe36196 node::OOMErrorHandler(char const*, v8::OOMDetails const&) [node]
 2: 0x123f4a0 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, v8::OOMDetails const&) [node]
 3: 0x123f777 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, v8::OOMDetails const&) [node]
 4: 0x146d1a5  [node]
 5: 0x146d1d3  [node]
 6: 0x148628a  [node]
 7: 0x1489458  [node]
 8: 0x1cc6071  [node]
Aborted (core dumped)

This happens because the line --- x\u2028x triggers the break on this line...

if ((/^(Index:\s|diff\s|---\s|\+\+\+\s|===================================================================)/).test(line)) {
        break;

... but does NOT match the regex at the start of parseFileHeader:

const fileHeader = (/^(---|\+\+\+)\s+(.*)\r?$/).exec(diffstr[i]);
if (fileHeader) {
  ...

(The reason it doesn't match is again down to the nasty trap that the regex wildcard . does not match character \u2028.)

The logic for chomping through lines of the patch relies on those two regexes being consistent with each other; since they are not, parseIndex finishes execution without actually having chomped the line, and gets called again by the main while loop in parsePatch, resulting in the line list.push(index); near the start of parseIndex being called again, resulting in list growing larger on each call until an OOM eventually occurs.

Oops!

This PR fixes it.

@ExplodingCabbage ExplodingCabbage self-assigned this Jan 7, 2026
@ExplodingCabbage ExplodingCabbage changed the title Fix the second ReDOS Fix the second denial-of-service vulnerability in parsePatch Jan 7, 2026
@ExplodingCabbage ExplodingCabbage marked this pull request as ready for review January 7, 2026 20:00
@ExplodingCabbage ExplodingCabbage merged commit 15a1585 into master Jan 7, 2026
@ExplodingCabbage ExplodingCabbage deleted the fix-redos-2 branch January 7, 2026 20:03
@SSP6904
Copy link

SSP6904 commented Jan 15, 2026

Is there going to be a fix for AstroJS? This vulnerability patch doesn't work with the newest version of the framework.
image

Edit: Astro is working on resolving this issue and a new release will come out soon.

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.

2 participants