From 9436534e334c4f85e470669019e89b55a36dec7d Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 24 Apr 2020 21:12:32 +0200 Subject: [PATCH 1/5] src: fix invalid windowBits=8 gzip segfault `{ windowBits: 8 }` is legal for deflate streams but not gzip streams. Fix a nullptr dereference when formatting the error message. Bug introduced in commit c34eae5f88 ("zlib: refactor zlib internals") from September 2018. --- src/node_zlib.cc | 9 +++++++-- test/parallel/test-zlib.js | 7 +++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/node_zlib.cc b/src/node_zlib.cc index eacd710143a1b2..df73e10f3ca7fd 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -111,7 +111,12 @@ enum node_zlib_mode { struct CompressionError { CompressionError(const char* message, const char* code, int err) - : message(message), code(code), err(err) {} + : message(message) + , code(code) + , err(err) { + CHECK_NOT_NULL(message); + } + CompressionError() = default; const char* message = nullptr; @@ -997,7 +1002,7 @@ CompressionError ZlibContext::Init( if (err_ != Z_OK) { dictionary_.clear(); mode_ = NONE; - return ErrorForMessage(nullptr); + return ErrorForMessage("zlib error"); } return SetDictionary(); diff --git a/test/parallel/test-zlib.js b/test/parallel/test-zlib.js index 0d21cf6cc29e58..3af791c379a45f 100644 --- a/test/parallel/test-zlib.js +++ b/test/parallel/test-zlib.js @@ -27,6 +27,13 @@ const stream = require('stream'); const fs = require('fs'); const fixtures = require('../common/fixtures'); +// Should not segfault. +assert.throws(() => zlib.gzipSync(Buffer.alloc(0), { windowBits: 8 }), { + code: 'ERR_ZLIB_INITIALIZATION_FAILED', + name: 'Error', + message: 'Initialization failed', +}); + let zlibPairs = [ [zlib.Deflate, zlib.Inflate], [zlib.Gzip, zlib.Gunzip], From 1cec86e9622c8cc5e27e986981a5b2dd51b65a2e Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 24 Apr 2020 21:12:32 +0200 Subject: [PATCH 2/5] zlib: reject windowBits=8 when mode=GZIP It's also handled in C++ land now, per the previous commit, but intercepting it in JS land makes for prettier error messages. --- lib/zlib.js | 4 +++- test/parallel/test-zlib-failed-init.js | 2 +- test/parallel/test-zlib-zero-windowBits.js | 2 +- test/parallel/test-zlib.js | 7 ++++--- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/zlib.js b/lib/zlib.js index 502ee61aa3bd9d..5027c96b996b46 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -613,9 +613,11 @@ function Zlib(opts, mode) { mode === UNZIP)) { windowBits = 0; } else { + // `{ windowBits: 8 }` is valid for deflate but nog gzip. + const min = Z_MIN_WINDOWBITS + (mode === GZIP); windowBits = checkRangesOrGetDefault( opts.windowBits, 'options.windowBits', - Z_MIN_WINDOWBITS, Z_MAX_WINDOWBITS, Z_DEFAULT_WINDOWBITS); + min, Z_MAX_WINDOWBITS, Z_DEFAULT_WINDOWBITS); } level = checkRangesOrGetDefault( diff --git a/test/parallel/test-zlib-failed-init.js b/test/parallel/test-zlib-failed-init.js index 1f99a378fc6eda..95f401a3718f30 100644 --- a/test/parallel/test-zlib-failed-init.js +++ b/test/parallel/test-zlib-failed-init.js @@ -21,7 +21,7 @@ assert.throws( code: 'ERR_OUT_OF_RANGE', name: 'RangeError', message: 'The value of "options.windowBits" is out of range. It must ' + - 'be >= 8 and <= 15. Received 0' + 'be >= 9 and <= 15. Received 0' } ); diff --git a/test/parallel/test-zlib-zero-windowBits.js b/test/parallel/test-zlib-zero-windowBits.js index 730690a07c423f..a27fd6734a5425 100644 --- a/test/parallel/test-zlib-zero-windowBits.js +++ b/test/parallel/test-zlib-zero-windowBits.js @@ -28,6 +28,6 @@ const zlib = require('zlib'); code: 'ERR_OUT_OF_RANGE', name: 'RangeError', message: 'The value of "options.windowBits" is out of range. ' + - 'It must be >= 8 and <= 15. Received 0' + 'It must be >= 9 and <= 15. Received 0' }); } diff --git a/test/parallel/test-zlib.js b/test/parallel/test-zlib.js index 3af791c379a45f..65050b85a036cf 100644 --- a/test/parallel/test-zlib.js +++ b/test/parallel/test-zlib.js @@ -29,9 +29,10 @@ const fixtures = require('../common/fixtures'); // Should not segfault. assert.throws(() => zlib.gzipSync(Buffer.alloc(0), { windowBits: 8 }), { - code: 'ERR_ZLIB_INITIALIZATION_FAILED', - name: 'Error', - message: 'Initialization failed', + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "options.windowBits" is out of range. ' + + 'It must be >= 9 and <= 15. Received 8', }); let zlibPairs = [ From 4de03b171788dd68a475fc72c7e9d56a688c22cf Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 8 May 2020 01:39:04 +0200 Subject: [PATCH 3/5] fixup! zlib: reject windowBits=8 when mode=GZIP --- lib/zlib.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/zlib.js b/lib/zlib.js index 5027c96b996b46..d9d9ba6315b92a 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -614,7 +614,7 @@ function Zlib(opts, mode) { windowBits = 0; } else { // `{ windowBits: 8 }` is valid for deflate but nog gzip. - const min = Z_MIN_WINDOWBITS + (mode === GZIP); + const min = Z_MIN_WINDOWBITS + (mode === GZIP ? 1 : 0); windowBits = checkRangesOrGetDefault( opts.windowBits, 'options.windowBits', min, Z_MAX_WINDOWBITS, Z_DEFAULT_WINDOWBITS); From 40da66df3a89ed13f702cb3d38d4a3e07ba1ab29 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 8 May 2020 01:39:19 +0200 Subject: [PATCH 4/5] fixup! src: fix invalid windowBits=8 gzip segfault --- src/node_zlib.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/node_zlib.cc b/src/node_zlib.cc index df73e10f3ca7fd..83698bd5192e4b 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -111,9 +111,9 @@ enum node_zlib_mode { struct CompressionError { CompressionError(const char* message, const char* code, int err) - : message(message) - , code(code) - , err(err) { + : message(message), + code(code), + err(err) { CHECK_NOT_NULL(message); } From 61b08c3293e0c4b0c87067722b129ccbdd8884ed Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 8 May 2020 03:42:47 +0200 Subject: [PATCH 5/5] fixup! zlib: reject windowBits=8 when mode=GZIP Co-authored-by: Jiawen Geng --- lib/zlib.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/zlib.js b/lib/zlib.js index d9d9ba6315b92a..be8f9401a310d0 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -613,7 +613,7 @@ function Zlib(opts, mode) { mode === UNZIP)) { windowBits = 0; } else { - // `{ windowBits: 8 }` is valid for deflate but nog gzip. + // `{ windowBits: 8 }` is valid for deflate but not gzip. const min = Z_MIN_WINDOWBITS + (mode === GZIP ? 1 : 0); windowBits = checkRangesOrGetDefault( opts.windowBits, 'options.windowBits',