Skip to content

Commit 237b450

Browse files
committed
zlib: reject trailing gzip members in web streams
Pass the existing rejectGarbageAfterEnd option through to the native zlib context and skip gunzip's concatenated-member loop when it is set. This lets DecompressionStream reject a second gzip member as trailing input while preserving default zlib gunzip behavior. Also make the sync zlib path honor rejectGarbageAfterEnd when native decompression leaves unused input, covering Brotli as well. Fixes: #58247 Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent 81e93df commit 237b450

3 files changed

Lines changed: 80 additions & 28 deletions

File tree

lib/zlib.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,11 @@ function processChunkSync(self, chunk, flushFlag) {
472472
}
473473
}
474474

475+
if (availInAfter > 0 && self._rejectGarbageAfterEnd) {
476+
_close(self);
477+
throw new ERR_TRAILING_JUNK_AFTER_STREAM_END();
478+
}
479+
475480
self.bytesWritten = inputRead;
476481
_close(self);
477482

@@ -678,7 +683,8 @@ function Zlib(opts, mode) {
678683
strategy,
679684
this._writeState,
680685
processCallback,
681-
dictionary);
686+
dictionary,
687+
opts?.rejectGarbageAfterEnd === true);
682688

683689
ZlibBase.call(this, opts, mode, handle, zlibDefaultOpts);
684690

src/node_zlib.cc

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ class ZlibContext final : public MemoryRetainer {
196196
int window_bits,
197197
int mem_level,
198198
int strategy,
199+
bool reject_garbage_after_end,
199200
std::vector<unsigned char>&& dictionary);
200201
CompressionError SetParams(int level, int strategy);
201202

@@ -223,6 +224,7 @@ class ZlibContext final : public MemoryRetainer {
223224
node_zlib_mode mode_ = NONE;
224225
int strategy_ = 0;
225226
int window_bits_ = 0;
227+
bool reject_garbage_after_end_ = false;
226228
unsigned int gzip_id_bytes_read_ = 0;
227229
std::vector<unsigned char> dictionary_;
228230

@@ -749,9 +751,10 @@ class ZlibStream final : public CompressionStream<ZlibContext> {
749751
"a version of npm (> 5.5.1 or < 5.4.0) or node-tar (> 4.0.1) "
750752
"that is compatible with Node.js 9 and above.\n");
751753
}
752-
CHECK(args.Length() == 7 &&
753-
"init(windowBits, level, memLevel, strategy, writeResult, writeCallback,"
754-
" dictionary)");
754+
CHECK((args.Length() == 7 || args.Length() == 8) &&
755+
"init(windowBits, level, memLevel, strategy, writeResult, "
756+
"writeCallback,"
757+
" dictionary[, rejectGarbageAfterEnd])");
755758

756759
ZlibStream* wrap;
757760
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This());
@@ -791,10 +794,20 @@ class ZlibStream final : public CompressionStream<ZlibContext> {
791794
data + Buffer::Length(args[6]));
792795
}
793796

797+
bool reject_garbage_after_end = false;
798+
if (args.Length() == 8) {
799+
CHECK(args[7]->IsBoolean());
800+
reject_garbage_after_end = args[7]->IsTrue();
801+
}
802+
794803
wrap->InitStream(write_result, write_js_callback);
795804

796805
AllocScope alloc_scope(wrap);
797-
wrap->context()->Init(level, window_bits, mem_level, strategy,
806+
wrap->context()->Init(level,
807+
window_bits,
808+
mem_level,
809+
strategy,
810+
reject_garbage_after_end,
798811
std::move(dictionary));
799812
}
800813

@@ -1124,10 +1137,8 @@ void ZlibContext::DoThreadPoolWork() {
11241137
}
11251138
}
11261139

1127-
while (strm_.avail_in > 0 &&
1128-
mode_ == GUNZIP &&
1129-
err_ == Z_STREAM_END &&
1130-
strm_.next_in[0] != 0x00) {
1140+
while (strm_.avail_in > 0 && mode_ == GUNZIP && err_ == Z_STREAM_END &&
1141+
!reject_garbage_after_end_ && strm_.next_in[0] != 0x00) {
11311142
// Bytes remain in input buffer. Perhaps this is another compressed
11321143
// member in the same archive, or just trailing garbage.
11331144
// Trailing zero bytes are okay, though, since they are frequently
@@ -1226,9 +1237,12 @@ CompressionError ZlibContext::ResetStream() {
12261237
return SetDictionary();
12271238
}
12281239

1229-
void ZlibContext::Init(
1230-
int level, int window_bits, int mem_level, int strategy,
1231-
std::vector<unsigned char>&& dictionary) {
1240+
void ZlibContext::Init(int level,
1241+
int window_bits,
1242+
int mem_level,
1243+
int strategy,
1244+
bool reject_garbage_after_end,
1245+
std::vector<unsigned char>&& dictionary) {
12321246
// Set allocation functions
12331247
strm_.zalloc = CompressionStreamMemoryOwner::AllocForZlib;
12341248
strm_.zfree = CompressionStreamMemoryOwner::FreeForZlib;
@@ -1259,6 +1273,7 @@ void ZlibContext::Init(
12591273
window_bits_ = window_bits;
12601274
mem_level_ = mem_level;
12611275
strategy_ = strategy;
1276+
reject_garbage_after_end_ = reject_garbage_after_end;
12621277

12631278
flush_ = Z_NO_FLUSH;
12641279

test/parallel/test-zlib-type-error.js

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,36 +2,67 @@
22
require('../common');
33
const assert = require('assert');
44
const test = require('node:test');
5+
const zlib = require('zlib');
56
const { DecompressionStream } = require('stream/web');
67

7-
test('DecompressStream deflat emits error on trailing data', async () => {
8+
async function assertDecompressionStreamRejects(format, chunks) {
9+
await assert.rejects(
10+
Array.fromAsync(
11+
new Blob(chunks).stream().pipeThrough(new DecompressionStream(format))
12+
),
13+
{ name: 'TypeError' },
14+
);
15+
}
16+
17+
test('DecompressionStream deflate emits TypeError on trailing data', async () => {
818
const valid = new Uint8Array([120, 156, 75, 4, 0, 0, 98, 0, 98]); // deflate('a')
919
const empty = new Uint8Array(1);
1020
const invalid = new Uint8Array([...valid, ...empty]);
1121
const double = new Uint8Array([...valid, ...valid]);
1222

13-
for (const chunk of [[invalid], [valid, empty], [valid, valid], [valid, double]]) {
14-
await assert.rejects(
15-
Array.fromAsync(
16-
new Blob([chunk]).stream().pipeThrough(new DecompressionStream('deflate'))
17-
),
18-
{ name: 'TypeError' },
19-
);
23+
for (const chunks of [[invalid], [valid, empty], [valid, valid], [double]]) {
24+
await assertDecompressionStreamRejects('deflate', chunks);
2025
}
2126
});
2227

23-
test('DecompressStream gzip emits error on trailing data', async () => {
28+
test('DecompressionStream gzip emits TypeError on trailing data', async () => {
2429
const valid = new Uint8Array([31, 139, 8, 0, 0, 0, 0, 0, 0, 19, 75, 4,
2530
0, 67, 190, 183, 232, 1, 0, 0, 0]); // gzip('a')
2631
const empty = new Uint8Array(1);
2732
const invalid = new Uint8Array([...valid, ...empty]);
2833
const double = new Uint8Array([...valid, ...valid]);
29-
for (const chunk of [[invalid], [valid, empty], [valid, valid], [double]]) {
30-
await assert.rejects(
31-
Array.fromAsync(
32-
new Blob([chunk]).stream().pipeThrough(new DecompressionStream('gzip'))
33-
),
34-
{ name: 'TypeError' },
35-
);
34+
for (const chunks of [[invalid], [valid, empty], [valid, valid], [double]]) {
35+
await assertDecompressionStreamRejects('gzip', chunks);
36+
}
37+
});
38+
39+
test('DecompressionStream brotli emits TypeError on trailing data', async () => {
40+
const valid = zlib.brotliCompressSync(Buffer.from('a'));
41+
const empty = new Uint8Array(1);
42+
const invalid = new Uint8Array([...valid, ...empty]);
43+
const double = new Uint8Array([...valid, ...valid]);
44+
for (const chunks of [[invalid], [valid, empty], [valid, valid], [double]]) {
45+
await assertDecompressionStreamRejects('brotli', chunks);
3646
}
3747
});
48+
49+
test('zlib sync decompression honors rejectGarbageAfterEnd', () => {
50+
const valid = new Uint8Array([31, 139, 8, 0, 0, 0, 0, 0, 0, 19, 75, 4,
51+
0, 67, 190, 183, 232, 1, 0, 0, 0]); // gzip('a')
52+
const double = new Uint8Array([...valid, ...valid]);
53+
54+
assert.deepStrictEqual(zlib.gunzipSync(double), Buffer.from('aa'));
55+
assert.throws(
56+
() => zlib.gunzipSync(double, { rejectGarbageAfterEnd: true }),
57+
{ code: 'ERR_TRAILING_JUNK_AFTER_STREAM_END', name: 'TypeError' },
58+
);
59+
60+
const brotli = zlib.brotliCompressSync(Buffer.from('a'));
61+
const brotliDouble = Buffer.concat([brotli, brotli]);
62+
63+
assert.deepStrictEqual(zlib.brotliDecompressSync(brotliDouble), Buffer.from('a'));
64+
assert.throws(
65+
() => zlib.brotliDecompressSync(brotliDouble, { rejectGarbageAfterEnd: true }),
66+
{ code: 'ERR_TRAILING_JUNK_AFTER_STREAM_END', name: 'TypeError' },
67+
);
68+
});

0 commit comments

Comments
 (0)