Skip to content

Commit

Permalink
zlib: fix node crashing on invalid options
Browse files Browse the repository at this point in the history
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: nodejs#13098
Fixes: nodejs#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>
  • Loading branch information
aqrln committed May 21, 2017
1 parent 6fb27af commit 9e4660b
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 15 deletions.
4 changes: 4 additions & 0 deletions doc/api/zlib.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 24 additions & 10 deletions lib/zlib.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);

Expand Down
13 changes: 8 additions & 5 deletions src/node_zlib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
37 changes: 37 additions & 0 deletions test/parallel/test-zlib-failed-init.js
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit 9e4660b

Please sign in to comment.