Skip to content

Commit e334d30

Browse files
panvaaduh95
authored andcommitted
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> PR-URL: #64023 Fixes: #58247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 8edf3ab commit e334d30

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

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

760763
ZlibStream* wrap;
761764
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This());
@@ -795,10 +798,20 @@ class ZlibStream final : public CompressionStream<ZlibContext> {
795798
data + Buffer::Length(args[6]));
796799
}
797800

801+
bool reject_garbage_after_end = false;
802+
if (args.Length() == 8) {
803+
CHECK(args[7]->IsBoolean());
804+
reject_garbage_after_end = args[7]->IsTrue();
805+
}
806+
798807
wrap->InitStream(write_result, write_js_callback);
799808

800809
AllocScope alloc_scope(wrap);
801-
wrap->context()->Init(level, window_bits, mem_level, strategy,
810+
wrap->context()->Init(level,
811+
window_bits,
812+
mem_level,
813+
strategy,
814+
reject_garbage_after_end,
802815
std::move(dictionary));
803816
}
804817

@@ -1128,10 +1141,8 @@ void ZlibContext::DoThreadPoolWork() {
11281141
}
11291142
}
11301143

1131-
while (strm_.avail_in > 0 &&
1132-
mode_ == GUNZIP &&
1133-
err_ == Z_STREAM_END &&
1134-
strm_.next_in[0] != 0x00) {
1144+
while (strm_.avail_in > 0 && mode_ == GUNZIP && err_ == Z_STREAM_END &&
1145+
!reject_garbage_after_end_ && strm_.next_in[0] != 0x00) {
11351146
// Bytes remain in input buffer. Perhaps this is another compressed
11361147
// member in the same archive, or just trailing garbage.
11371148
// Trailing zero bytes are okay, though, since they are frequently
@@ -1230,9 +1241,12 @@ CompressionError ZlibContext::ResetStream() {
12301241
return SetDictionary();
12311242
}
12321243

1233-
void ZlibContext::Init(
1234-
int level, int window_bits, int mem_level, int strategy,
1235-
std::vector<unsigned char>&& dictionary) {
1244+
void ZlibContext::Init(int level,
1245+
int window_bits,
1246+
int mem_level,
1247+
int strategy,
1248+
bool reject_garbage_after_end,
1249+
std::vector<unsigned char>&& dictionary) {
12361250
// Set allocation functions
12371251
strm_.zalloc = CompressionStreamMemoryOwner::AllocForZlib;
12381252
strm_.zfree = CompressionStreamMemoryOwner::FreeForZlib;
@@ -1263,6 +1277,7 @@ void ZlibContext::Init(
12631277
window_bits_ = window_bits;
12641278
mem_level_ = mem_level;
12651279
strategy_ = strategy;
1280+
reject_garbage_after_end_ = reject_garbage_after_end;
12661281

12671282
flush_ = Z_NO_FLUSH;
12681283

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)