Skip to content

zlib: add error handling for trailing junk after stream end #58316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -3012,6 +3012,15 @@ category.
The `node:trace_events` module could not be loaded because Node.js was compiled
with the `--without-v8-platform` flag.

<a id="ERR_TRAILING_JUNK_AFTER_STREAM_END"></a>

### `ERR_TRAILING_JUNK_AFTER_STREAM_END`

Trailing junk found after the end of the compressed stream.
This error is thrown when extra, unexpected data is detected
after the end of a compressed stream (for example, in zlib
or gzip decompression).

<a id="ERR_TRANSFORM_ALREADY_TRANSFORMING"></a>

### `ERR_TRANSFORM_ALREADY_TRANSFORMING`
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1803,6 +1803,8 @@ E('ERR_TRACE_EVENTS_CATEGORY_REQUIRED',
'At least one category is required', TypeError);
E('ERR_TRACE_EVENTS_UNAVAILABLE', 'Trace events are unavailable', Error);

E('ERR_TRAILING_JUNK_AFTER_STREAM_END', 'Trailing junk found after the end of the compressed stream', TypeError);

// This should probably be a `RangeError`.
E('ERR_TTY_INIT_FAILED', 'TTY initialization failed', SystemError);
E('ERR_UNAVAILABLE_DURING_EXIT', 'Cannot call function in process exit ' +
Expand Down
16 changes: 14 additions & 2 deletions lib/internal/webstreams/compression.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,28 @@ class DecompressionStream {
});
switch (format) {
case 'deflate':
this.#handle = lazyZlib().createInflate();
this.#handle = lazyZlib().createInflate({
rejectGarbageAfterEnd: true,
});
break;
case 'deflate-raw':
this.#handle = lazyZlib().createInflateRaw();
break;
case 'gzip':
this.#handle = lazyZlib().createGunzip();
this.#handle = lazyZlib().createGunzip({
rejectGarbageAfterEnd: true,
});
break;
}
this.#transform = newReadableWritablePairFromDuplex(this.#handle);

this.#handle.on('error', (err) => {
if (this.#transform?.writable &&
!this.#transform.writable.locked &&
typeof this.#transform.writable.abort === 'function') {
this.#transform.writable.abort(err);
}
});
}

/**
Expand Down
12 changes: 12 additions & 0 deletions lib/zlib.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const {
ERR_BUFFER_TOO_LARGE,
ERR_INVALID_ARG_TYPE,
ERR_OUT_OF_RANGE,
ERR_TRAILING_JUNK_AFTER_STREAM_END,
ERR_ZSTD_INVALID_PARAM,
},
genericNodeError,
Expand Down Expand Up @@ -266,6 +267,8 @@ function ZlibBase(opts, mode, handle, { flush, finishFlush, fullFlush }) {
this._defaultFullFlushFlag = fullFlush;
this._info = opts?.info;
this._maxOutputLength = maxOutputLength;

this._rejectGarbageAfterEnd = opts?.rejectGarbageAfterEnd === true;
}
ObjectSetPrototypeOf(ZlibBase.prototype, Transform.prototype);
ObjectSetPrototypeOf(ZlibBase, Transform);
Expand Down Expand Up @@ -570,6 +573,14 @@ function processCallback() {
// stream has ended early.
// This applies to streams where we don't check data past the end of
// what was consumed; that is, everything except Gunzip/Unzip.

if (self._rejectGarbageAfterEnd) {
const err = new ERR_TRAILING_JUNK_AFTER_STREAM_END();
self.destroy(err);
this.cb(err);
return;
}

self.push(null);
}

Expand Down Expand Up @@ -662,6 +673,7 @@ function Zlib(opts, mode) {

this._level = level;
this._strategy = strategy;
this._mode = mode;
}
ObjectSetPrototypeOf(Zlib.prototype, ZlibBase.prototype);
ObjectSetPrototypeOf(Zlib, ZlibBase);
Expand Down
46 changes: 46 additions & 0 deletions test/parallel/test-zlib-type-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';
require('../common');
const assert = require('assert').strict;
const test = require('node:test');
const { DecompressionStream } = require('stream/web');

async function expectTypeError(promise) {
let threw = false;
try {
await promise;
} catch (err) {
threw = true;
assert(err instanceof TypeError, `Expected TypeError, got ${err}`);
}
assert(threw, 'Expected promise to reject');
}

test('DecompressStream deflat emits error on trailing data', async () => {
const valid = new Uint8Array([120, 156, 75, 4, 0, 0, 98, 0, 98]); // deflate('a')
const empty = new Uint8Array(1);
const invalid = new Uint8Array([...valid, ...empty]);
const double = new Uint8Array([...valid, ...valid]);

for (const chunk of [[invalid], [valid, empty], [valid, valid], [valid, double]]) {
await expectTypeError(
Array.fromAsync(
new Blob([chunk]).stream().pipeThrough(new DecompressionStream('deflate'))
)
);
}
});

test('DecompressStream gzip emits error on trailing data', async () => {
const valid = new Uint8Array([31, 139, 8, 0, 0, 0, 0, 0, 0, 19, 75, 4,
0, 67, 190, 183, 232, 1, 0, 0, 0]); // gzip('a')
const empty = new Uint8Array(1);
const invalid = new Uint8Array([...valid, ...empty]);
const double = new Uint8Array([...valid, ...valid]);
for (const chunk of [[invalid], [valid, empty], [valid, valid], [double]]) {
await expectTypeError(
Array.fromAsync(
new Blob([chunk]).stream().pipeThrough(new DecompressionStream('gzip'))
)
);
}
});
8 changes: 0 additions & 8 deletions test/wpt/status/compression.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,6 @@
"compression-with-detach.tentative.window.js": {
"requires": ["crypto"]
},
"decompression-corrupt-input.tentative.any.js": {
"fail": {
"expected": [
"trailing junk for 'deflate' should give an error",
"trailing junk for 'gzip' should give an error"
]
}
},
"idlharness-shadowrealm.window.js": {
"skip": "ShadowRealm support is not enabled"
},
Expand Down
Loading