Skip to content

src: fix edge case when deflateInit2() fails with Z_VERSION_ERROR#63476

Open
ndossche wants to merge 1 commit into
nodejs:mainfrom
ndossche:zlib-1
Open

src: fix edge case when deflateInit2() fails with Z_VERSION_ERROR#63476
ndossche wants to merge 1 commit into
nodejs:mainfrom
ndossche:zlib-1

Conversation

@ndossche
Copy link
Copy Markdown
Contributor

@ndossche ndossche commented May 21, 2026

deflateInit2() can fail with Z_VERSION_ERROR if the compiled library vs loaded library mismatched in version number or in stream structure size.
In those cases, zlib doesn't initialize the strm_.msg field to null. Therefore, when a CompressionError object is created via ErrorForMessage(), it can read a stale or uninitialized strm_.msg pointer that will cause a crash.

Example ASAN report:

==291205==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x7222adf1a7dd bp 0x7fff2de70650 sp 0x7fff2de6fe08 T0)
==291205==The signal is caused by a READ memory access.
==291205==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
    #0 0x7222adf1a7dd in __strlen_avx2 string/../sysdeps/x86_64/multiarch/strlen-avx2.S:76
    #1 0x5bf61d442ab7 in strlen (/work/node/out/Debug/node+0x1a42ab7) (BuildId: 54c4d388769aa50da2f368749b0abbbb804e86a9)
    #2 0x5bf61e2d18c4 in v8::(anonymous namespace)::StringLength(char const*) /work/node/out/../deps/v8/src/api/api.cc:7581:16
    #3 0x5bf61e2d18c4 in v8::(anonymous namespace)::StringLength(unsigned char const*) /work/node/out/../deps/v8/src/api/api.cc:7587:10
    #4 0x5bf61e2d18c4 in v8::String::NewFromOneByte(v8::Isolate*, unsigned char const*, v8::NewStringType, int) /work/node/out/../deps/v8/src/api/api.cc:7677:3
    #5 0x5bf61d5499b8 in node::OneByteString(v8::Isolate*, char const*, int, v8::NewStringType) /work/node/out/../src/util-inl.h:166:10
    #6 0x5bf61de71213 in node::(anonymous namespace)::CompressionStream<node::(anonymous namespace)::ZlibContext>::EmitError(node::(anonymous namespace)::CompressionError const&) /work/node/out/../src/node_zlib.cc:565:7
    #7 0x5bf61de70be6 in node::(anonymous namespace)::CompressionStream<node::(anonymous namespace)::ZlibContext>::CheckError() /work/node/out/../src/node_zlib.cc:519:5
    #8 0x5bf61de6d9c0 in node::(anonymous namespace)::CompressionStream<node::(anonymous namespace)::ZlibContext>::AfterThreadPoolWork(int) /work/node/out/../src/node_zlib.cc:543:10
    #9 0x5bf61d8729be in node::ThreadPoolWork::ScheduleWork()::'lambda'(uv_work_s*, int)::operator()(uv_work_s*, int) const /work/node/out/../src/threadpoolwork-inl.h:57:15
    #10 0x5bf61d87242c in node::ThreadPoolWork::ScheduleWork()::'lambda'(uv_work_s*, int)::__invoke(uv_work_s*, int) /work/node/out/../src/threadpoolwork-inl.h:48:7
    #11 0x7222aea8bfef in uv__work_done /work/libuv-1.51.0/src/threadpool.c:330:5
    #12 0x7222aea900e2 in uv__async_io.part.0 /work/libuv-1.51.0/src/unix/async.c:208:5

@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 May 21, 2026
Comment thread src/node_zlib.cc Outdated

CompressionError ZlibContext::ErrorForMessage(const char* message) const {
if (strm_.msg != nullptr)
if (zlib_init_done_ && strm_.msg != nullptr)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That means that no error from InitZlib() would be reported, which doesn't seem particularly desirable.

Can we instead pre-initialize strm_.msg = nullptr; to avoid this issue?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works too. Did that now. Thanks.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.07%. Comparing base (5c87012) to head (c6ea900).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #63476   +/-   ##
=======================================
  Coverage   90.07%   90.07%           
=======================================
  Files         714      714           
  Lines      225935   225953   +18     
  Branches    42738    42739    +1     
=======================================
+ Hits       203503   203529   +26     
+ Misses      14206    14204    -2     
+ Partials     8226     8220    -6     
Files with missing lines Coverage Δ
src/node_zlib.cc 78.32% <100.00%> (+0.37%) ⬆️

... and 34 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.

This function call can fail with `Z_VERSION_ERROR` if the compiled
library vs loaded library mismatched in version number or in
stream structure size.
In those cases, zlib doesn't initialize the `strm_.msg` field to
null. Therefore, when a `CompressionError` object is created via
`ErrorForMessage()`, it can read a stale or uninitialized `strm_.msg`
pointer that will cause a crash.

Example ASAN report:
```
AddressSanitizer: SEGV on unknown address
    #0 __strlen_avx2
        string/../sysdeps/x86_64/multiarch/strlen-avx2.S:76
    nodejs#1 strlen (/work/node/out/Debug/node+0x1a42ab7)
    nodejs#2 v8::(anonymous namespace)::StringLength(char const*)
        /work/node/out/../deps/v8/src/api/api.cc:7581:16
    nodejs#3 v8::(anonymous namespace)::StringLength(unsigned char const*)
        /work/node/out/../deps/v8/src/api/api.cc:7587:10
    nodejs#4 v8::String::NewFromOneByte(v8::Isolate*,
        unsigned char const*, v8::NewStringType, int)
        /work/node/out/../deps/v8/src/api/api.cc:7677:3
    nodejs#5 node::OneByteString(v8::Isolate*,
        char const*, int, v8::NewStringType)
        /work/node/out/../src/util-inl.h:166:10
    nodejs#6 node::(anonymous namespace)::CompressionStream<
        node::(anonymous namespace)::ZlibContext>
        ::EmitError(node::(anonymous namespace)
        ::CompressionError const&)
        /work/node/out/../src/node_zlib.cc:565:7
    nodejs#7 node::(anonymous namespace)::CompressionStream<
        node::(anonymous namespace)::ZlibContext>
        ::CheckError()
        /work/node/out/../src/node_zlib.cc:519:5
    nodejs#8 node::(anonymous namespace)::CompressionStream<
        node::(anonymous namespace)::ZlibContext>
        ::AfterThreadPoolWork(int)
        /work/node/out/../src/node_zlib.cc:543:10
    nodejs#9 node::ThreadPoolWork::ScheduleWork()
        ::'lambda'(uv_work_s*, int)
        ::operator()(uv_work_s*, int) const
        /work/node/out/../src/threadpoolwork-inl.h:57:15
    nodejs#10 node::ThreadPoolWork::ScheduleWork()
        ::'lambda'(uv_work_s*, int)
        ::__invoke(uv_work_s*, int)
        /work/node/out/../src/threadpoolwork-inl.h:48:7
    nodejs#11 uv__work_done /work/libuv-1.51.0/src/threadpool.c:330:5
    nodejs#12 uv__async_io.part.0
        /work/libuv-1.51.0/src/unix/async.c:208:5
```

Signed-off-by: ndossche <nora.dossche@ugent.be>
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.

3 participants