Skip to content

Conversation

thunder-coding
Copy link
Contributor

Calling V8::ExternalMemoryAccounter::Increase with a signed integer on
32-bit platforms causes instances where GC inside GC takes place leading
to a crash in certain cases.

During GC, native objects are destructed. In destructor for
CompressionStream class used by zlib, memory release information is
passed onto V8::ExternalMemoryAccounter::Increase() instead of
V8::ExternalMemoryAccounter::Decrease() which triggers V8's memory
limits, thus triggering GC inside GC which leads to crash.

Bug initially introduced in commit
1d5d7b6

For full report see https://hackerone.com/reports/3302484

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http2

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem. labels Aug 25, 2025
Call `V8::ExternalMemoryAccounter::Update` instead of
`V8::ExternalMemoryAccounter::Increase` to report memory difference to
V8

Calling `V8::ExternalMemoryAccounter::Increase` with a signed integer on
32-bit platforms causes instances where GC inside GC takes place leading
to a crash in certain cases.

During GC, native objects are destructed. In destructor for
`CompressionStream` class used by zlib, memory release information is
passed onto `V8::ExternalMemoryAccounter::Increase()` instead of
`V8::ExternalMemoryAccounter::Decrease()` which triggers V8's memory
limits, thus triggering GC inside GC which leads to crash.

Bug initially introduced in commit
1d5d7b6

For full report see https://hackerone.com/reports/3302484
thunder-coding added a commit to termux/termux-packages that referenced this pull request Aug 25, 2025
zlib.createDeflate().write()

The plan was to do a coordinated disclosure with upstream as it is a
crash that was happening on all 32-bit Linux binaries, but upstream is
not willing to block releases for failures for 32-bit Linux, so we'll
have to go ahead and patch this ourselves.

The fix has also been sent upstream as in nodejs/node#59623

Full writeup of the security report can be found on https://hackerone.com/reports/3302484

Closes #25455
termux-pacman-bot added a commit to termux-pacman/termux-packages that referenced this pull request Aug 25, 2025
zlib.createDeflate().write()

The plan was to do a coordinated disclosure with upstream as it is a
crash that was happening on all 32-bit Linux binaries, but upstream is
not willing to block releases for failures for 32-bit Linux, so we'll
have to go ahead and patch this ourselves.

The fix has also been sent upstream as in nodejs/node#59623

Full writeup of the security report can be found on https://hackerone.com/reports/3302484

Closes termux/termux-packages#25455
Copy link

codecov bot commented Aug 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.83%. Comparing base (6722642) to head (9d77c41).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59623      +/-   ##
==========================================
- Coverage   89.84%   89.83%   -0.02%     
==========================================
  Files         667      667              
  Lines      196260   196260              
  Branches    38563    38562       -1     
==========================================
- Hits       176332   176310      -22     
- Misses      12382    12416      +34     
+ Partials     7546     7534      -12     
Files with missing lines Coverage Δ
src/node_mem-inl.h 92.59% <100.00%> (+1.85%) ⬆️
src/node_zlib.cc 77.99% <100.00%> (ø)

... and 40 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants