Skip to content

Commit

Permalink
zlib: gracefully set windowBits from 8 to 9
Browse files Browse the repository at this point in the history
On 4 April 2017, Node.js versions v4.8.2 and v6.10.2 were
released. These versions bumped the vendored zlib library from
v1.2.8 to v1.2.11 in response to what it describes as low-severity
CVEs. In zlib v1.2.9, a change was made that causes an error to be
raised when a raw deflate stream is initialised with windowBits set
to 8.

In zlib v1.2.9, 8 become an invalid value for this parameter, and Node's zlib
module will crash if you call this:

```
zlib.createDeflateRaw({windowBits: 8})
```

On some versions this crashes Node and you cannot recover from it, while on some
versions it throws an exception. The permessage-deflate library up to
version v0.1.5 does make such a call with no try/catch

This commit reverts to the original behavior of zlib by gracefully changed
windowBits: 8 to windowBits: 9 for raw deflate streams.

Original-PR-URL: nodejs-private/node-private#95
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>

PR-URL: #16511
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
  • Loading branch information
MylesBorins authored and addaleax committed Oct 29, 2017
1 parent 203b548 commit 241eb61
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 20 deletions.
10 changes: 7 additions & 3 deletions doc/api/zlib.md
Original file line number Diff line number Diff line change
Expand Up @@ -458,9 +458,13 @@ added: v0.5.8

Creates and returns a new [DeflateRaw][] object with the given [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.
*Note*: An upgrade of zlib from 1.2.8 to 1.2.11 changed behavior when windowBits
is set to 8 for raw deflate streams. zlib does not have a working implementation
of an 8-bit Window for raw deflate streams and would automatically set windowBit
to 9 if initially set to 8. Newer versions of zlib will throw an exception.
This creates a potential DOS vector, and as such the behavior ahs been reverted
in Node.js 8, 6, and 4. Node.js version 9 and higher will throw when windowBits
is set to 8.

## zlib.createGunzip([options])
<!-- YAML
Expand Down
1 change: 1 addition & 0 deletions lib/zlib.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,7 @@ function Gunzip(opts) {
inherits(Gunzip, Zlib);

function DeflateRaw(opts) {
if (opts && opts.windowBits === 8) opts.windowBits = 9;
if (!(this instanceof DeflateRaw))
return new DeflateRaw(opts);
Zlib.call(this, opts, DEFLATERAW);
Expand Down
17 changes: 0 additions & 17 deletions test/parallel/test-zlib-failed-init.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,6 @@ const common = 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).
// This check was introduced in version 1.2.9 and prior to that there was
// no such rejection which is the reason for the version check below
// (http://zlib.net/ChangeLog.txt).
if (!/^1\.2\.[0-8]$/.test(process.versions.zlib)) {
common.expectsError(
() => zlib.createDeflateRaw({ windowBits: 8 }),
{
code: 'ERR_ZLIB_INITIALIZATION_FAILED',
type: Error,
message: 'Initialization failed'
});
}

// Regression tests for bugs in the validation logic.

common.expectsError(
() => zlib.createGzip({ chunkSize: 0 }),
{
Expand Down
23 changes: 23 additions & 0 deletions test/parallel/test-zlib.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const common = require('../common');
const assert = require('assert');
const zlib = require('zlib');
const stream = require('stream');
const fs = require('fs');
const fixtures = require('../common/fixtures');

let zlibPairs = [
Expand Down Expand Up @@ -150,6 +151,28 @@ class SlowStream extends stream.Stream {
}
}

// windowBits: 8 shouldn't throw
assert.doesNotThrow(() => {
zlib.createDeflateRaw({ windowBits: 8 });
}, 'windowsBits set to 8 should follow legacy zlib behavior');

{
const node = fs.createReadStream(process.execPath);
const raw = [];
const reinflated = [];
node.on('data', (chunk) => raw.push(chunk));

// Usually, the inflate windowBits parameter needs to be at least the
// value of the matching deflate’s windowBits. However, inflate raw with
// windowBits = 8 should be able to handle compressed data from a source
// that does not know about the silent 8-to-9 upgrade of windowBits
// that older versions of zlib/Node perform.
node.pipe(zlib.createDeflateRaw({ windowBits: 9 }))
.pipe(zlib.createInflateRaw({ windowBits: 8 }))
.on('data', (chunk) => reinflated.push(chunk))
.on('end', common.mustCall(
() => assert(Buffer.concat(raw).equals(Buffer.concat(reinflated)))));
}

// for each of the files, make sure that compressing and
// decompressing results in the same data, for every combination
Expand Down

2 comments on commit 241eb61

@Trott
Copy link
Member

@Trott Trott commented on 241eb61 Oct 29, 2017

Choose a reason for hiding this comment

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

FYI, commit message has lines longer than 72 chars. core-validate-commit is your friend. :-D

@jasnell
Copy link
Member

Choose a reason for hiding this comment

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

Not my fault this time! ;-) 😆

Please sign in to comment.