zlib: fix node crashing on invalid options

This commit fixes the Node process crashing when constructors of classes
of the zlib module are given invalid options.

* Throw an Error when the zlib library rejects the value of windowBits,
  instead of crashing with an assertion.

* Treat windowBits and memLevel options consistently with other ones and
  don't crash when non-numeric values are given.

* Fix bugs in the validation logic:
  - Don't conflate 0 and undefined when checking if a field of an
    options object exists.
  - Treat NaN and Infinity values the same way as values of invalid
    types instead of allowing to actually set zlib options to NaN or
    Infinity.

PR-URL: https://github.com/nodejs/node/pull/13098
Fixes: https://github.com/nodejs/node/issues/13082
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

4 files changed, 73 insertions, 15 deletions

diff --git a/doc/api/zlib.md b/doc/api/zlib.md
index abfb6e2038..f889914bb0 100644
--- a/doc/api/zlib.md
+++ b/doc/api/zlib.md
@@ -437,6 +437,10 @@ added: v0.5.8
 
 Returns a new [DeflateRaw][] object with an [options][].
 
+**Note:** The zlib library rejects requests for 256-byte windows (i.e.,
+`{ windowBits: 8 }` in `options`). An `Error` will be thrown when creating
+a [DeflateRaw][] object with this specific value of the `windowBits` option.
+
 ## zlib.createGunzip([options])
 <!-- YAML
 added: v0.5.8
diff --git a/lib/zlib.js b/lib/zlib.js
index 00be56dffc..c1cc0a2c54 100644
--- a/lib/zlib.js
+++ b/lib/zlib.js
@@ -182,37 +182,37 @@ class Zlib extends Transform {
     this._finishFlushFlag = opts.finishFlush !== undefined ?
       opts.finishFlush : constants.Z_FINISH;
 
-    if (opts.chunkSize) {
+    if (opts.chunkSize !== undefined) {
       if (opts.chunkSize < constants.Z_MIN_CHUNK) {
         throw new RangeError('Invalid chunk size: ' + opts.chunkSize);
       }
     }
 
-    if (opts.windowBits) {
+    if (opts.windowBits !== undefined) {
       if (opts.windowBits < constants.Z_MIN_WINDOWBITS ||
           opts.windowBits > constants.Z_MAX_WINDOWBITS) {
         throw new RangeError('Invalid windowBits: ' + opts.windowBits);
       }
     }
 
-    if (opts.level) {
+    if (opts.level !== undefined) {
       if (opts.level < constants.Z_MIN_LEVEL ||
           opts.level > constants.Z_MAX_LEVEL) {
         throw new RangeError('Invalid compression level: ' + opts.level);
       }
     }
 
-    if (opts.memLevel) {
+    if (opts.memLevel !== undefined) {
       if (opts.memLevel < constants.Z_MIN_MEMLEVEL ||
           opts.memLevel > constants.Z_MAX_MEMLEVEL) {
         throw new RangeError('Invalid memLevel: ' + opts.memLevel);
       }
     }
 
-    if (opts.strategy && isInvalidStrategy(opts.strategy))
+    if (opts.strategy !== undefined && isInvalidStrategy(opts.strategy))
       throw new TypeError('Invalid strategy: ' + opts.strategy);
 
-    if (opts.dictionary) {
+    if (opts.dictionary !== undefined) {
       if (!ArrayBuffer.isView(opts.dictionary)) {
         throw new TypeError(
           'Invalid dictionary: it should be a Buffer, TypedArray, or DataView');
@@ -224,14 +224,28 @@ class Zlib extends Transform {
     this._hadError = false;
 
     var level = constants.Z_DEFAULT_COMPRESSION;
-    if (typeof opts.level === 'number') level = opts.level;
+    if (Number.isFinite(opts.level)) {
+      level = opts.level;
+    }
 
     var strategy = constants.Z_DEFAULT_STRATEGY;
-    if (typeof opts.strategy === 'number') strategy = opts.strategy;
+    if (Number.isFinite(opts.strategy)) {
+      strategy = opts.strategy;
+    }
+
+    var windowBits = constants.Z_DEFAULT_WINDOWBITS;
+    if (Number.isFinite(opts.windowBits)) {
+      windowBits = opts.windowBits;
+    }
+
+    var memLevel = constants.Z_DEFAULT_MEMLEVEL;
+    if (Number.isFinite(opts.memLevel)) {
+      memLevel = opts.memLevel;
+    }
 
-    this._handle.init(opts.windowBits || constants.Z_DEFAULT_WINDOWBITS,
+    this._handle.init(windowBits,
                       level,
-                      opts.memLevel || constants.Z_DEFAULT_MEMLEVEL,
+                      memLevel,
                       strategy,
                       opts.dictionary);
 
diff --git a/src/node_zlib.cc b/src/node_zlib.cc
index e4adda5202..4495eb2bca 100644
--- a/src/node_zlib.cc
+++ b/src/node_zlib.cc
@@ -551,16 +551,19 @@ class ZCtx : public AsyncWrap {
         CHECK(0 && "wtf?");
     }
 
-    if (ctx->err_ != Z_OK) {
-      ZCtx::Error(ctx, "Init error");
-    }
-
-
     ctx->dictionary_ = reinterpret_cast<Bytef *>(dictionary);
     ctx->dictionary_len_ = dictionary_len;
 
     ctx->write_in_progress_ = false;
     ctx->init_done_ = true;
+
+    if (ctx->err_ != Z_OK) {
+      if (dictionary != nullptr) {
+        delete[] dictionary;
+        ctx->dictionary_ = nullptr;
+      }
+      ctx->env()->ThrowError("Init error");
+    }
   }
 
   static void SetDictionary(ZCtx* ctx) {
diff --git a/test/parallel/test-zlib-failed-init.js b/test/parallel/test-zlib-failed-init.js
new file mode 100644
index 0000000000..3b6da1d4fb
--- /dev/null
+++ b/test/parallel/test-zlib-failed-init.js
@@ -0,0 +1,37 @@
+'use strict';
+
+require('../common');
+
+const assert = require('assert');
+const zlib = require('zlib');
+
+// For raw deflate encoding, requests for 256-byte windows are rejected as
+// invalid by zlib.
+// (http://zlib.net/manual.html#Advanced)
+assert.throws(() => {
+  zlib.createDeflateRaw({ windowBits: 8 });
+}, /^Error: Init error$/);
+
+// Regression tests for bugs in the validation logic.
+
+assert.throws(() => {
+  zlib.createGzip({ chunkSize: 0 });
+}, /^RangeError: Invalid chunk size: 0$/);
+
+assert.throws(() => {
+  zlib.createGzip({ windowBits: 0 });
+}, /^RangeError: Invalid windowBits: 0$/);
+
+assert.throws(() => {
+  zlib.createGzip({ memLevel: 0 });
+}, /^RangeError: Invalid memLevel: 0$/);
+
+{
+  const stream = zlib.createGzip({ level: NaN });
+  assert.strictEqual(stream._level, zlib.constants.Z_DEFAULT_COMPRESSION);
+}
+
+{
+  const stream = zlib.createGzip({ strategy: NaN });
+  assert.strictEqual(stream._strategy, zlib.constants.Z_DEFAULT_STRATEGY);
+}