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: #64023Fixes: #58247
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Author: panva

lib/zlib.js

@@ -472,6 +472,11 @@ function processChunkSync(self, chunk, flushFlag) {
 }
 }
 
+if(availInAfter>0&&self._rejectGarbageAfterEnd){
+_close(self);
+thrownewERR_TRAILING_JUNK_AFTER_STREAM_END();
+}
+
 self.bytesWritten=inputRead;
 _close(self);
 
@@ -678,7 +683,8 @@ function Zlib(opts, mode) {
 strategy,
 this._writeState,
 processCallback,
-dictionary);
+dictionary,
+opts?.rejectGarbageAfterEnd===true);
 
 ZlibBase.call(this,opts,mode,handle,zlibDefaultOpts);
 

src/node_zlib.cc

@@ -196,6 +196,7 @@ class ZlibContext final : public MemoryRetainer {
 int window_bits,
 int mem_level,
 int strategy,
+bool reject_garbage_after_end,
  std::vector<unsignedchar>&& dictionary);
  CompressionError SetParams(int level, int strategy);
 
@@ -223,6 +224,7 @@ class ZlibContext final : public MemoryRetainer {
  node_zlib_mode mode_ = NONE;
 int strategy_ = 0;
 int window_bits_ = 0;
+bool reject_garbage_after_end_ = false;
 unsignedint gzip_id_bytes_read_ = 0;
  std::vector<unsignedchar> dictionary_;
 
@@ -749,9 +751,10 @@ class ZlibStream final : public CompressionStream<ZlibContext> {
 "a version of npm (> 5.5.1 or < 5.4.0) or node-tar (> 4.0.1) "
 "that is compatible with Node.js 9 and above.\n");
  }
-CHECK(args.Length() == 7 &&
-"init(windowBits, level, memLevel, strategy, writeResult, writeCallback,"
-" dictionary)");
+CHECK((args.Length() == 7 || args.Length() == 8) &&
+"init(windowBits, level, memLevel, strategy, writeResult, "
+"writeCallback,"
+" dictionary[, rejectGarbageAfterEnd])");
 
  ZlibStream* wrap;
 ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This());
@@ -791,10 +794,20 @@ class ZlibStream final : public CompressionStream<ZlibContext> {
  data + Buffer::Length(args[6]));
  }
 
+bool reject_garbage_after_end = false;
+if (args.Length() == 8) {
+CHECK(args[7]->IsBoolean());
+ reject_garbage_after_end = args[7]->IsTrue();
+ }
+
  wrap->InitStream(write_result, write_js_callback);
 
  AllocScope alloc_scope(wrap);
- wrap->context()->Init(level, window_bits, mem_level, strategy,
+ wrap->context()->Init(level,
+ window_bits,
+ mem_level,
+ strategy,
+ reject_garbage_after_end,
 std::move(dictionary));
  }
 
@@ -1124,10 +1137,8 @@ void ZlibContext::DoThreadPoolWork() {
  }
  }
 
-while (strm_.avail_in > 0 &&
- mode_ == GUNZIP &&
- err_ == Z_STREAM_END &&
- strm_.next_in[0] != 0x00) {
+while (strm_.avail_in > 0 && mode_ == GUNZIP && err_ == Z_STREAM_END &&
+ !reject_garbage_after_end_ && strm_.next_in[0] != 0x00) {
 // Bytes remain in input buffer. Perhaps this is another compressed
 // member in the same archive, or just trailing garbage.
 // Trailing zero bytes are okay, though, since they are frequently
@@ -1226,9 +1237,12 @@ CompressionError ZlibContext::ResetStream() {
 returnSetDictionary();
 }
 
-voidZlibContext::Init(
-int level, int window_bits, int mem_level, int strategy,
- std::vector<unsignedchar>&& dictionary) {
+voidZlibContext::Init(int level,
+int window_bits,
+int mem_level,
+int strategy,
+bool reject_garbage_after_end,
+ std::vector<unsignedchar>&& dictionary) {
 // Set allocation functions
  strm_.zalloc = CompressionStreamMemoryOwner::AllocForZlib;
  strm_.zfree = CompressionStreamMemoryOwner::FreeForZlib;
@@ -1259,6 +1273,7 @@ void ZlibContext::Init(
  window_bits_ = window_bits;
  mem_level_ = mem_level;
  strategy_ = strategy;
+ reject_garbage_after_end_ = reject_garbage_after_end;
 
  flush_ = Z_NO_FLUSH;
 

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

@@ -2,36 +2,67 @@
 require('../common');
 constassert=require('assert');
 consttest=require('node:test');
+constzlib=require('zlib');
 const{ DecompressionStream }=require('stream/web');
 
-test('DecompressStream deflat emits error on trailing data',async()=>{
+asyncfunctionassertDecompressionStreamRejects(format,chunks){
+awaitassert.rejects(
+Array.fromAsync(
+newBlob(chunks).stream().pipeThrough(newDecompressionStream(format))
+),
+{name: 'TypeError'},
+);
+}
+
+test('DecompressionStream deflate emits TypeError on trailing data',async()=>{
 constvalid=newUint8Array([120,156,75,4,0,0,98,0,98]);// deflate('a')
 constempty=newUint8Array(1);
 constinvalid=newUint8Array([...valid, ...empty]);
 constdouble=newUint8Array([...valid, ...valid]);
 
-for(constchunkof[[invalid],[valid,empty],[valid,valid],[valid,double]]){
-awaitassert.rejects(
-Array.fromAsync(
-newBlob([chunk]).stream().pipeThrough(newDecompressionStream('deflate'))
-),
-{name: 'TypeError'},
-);
+for(constchunksof[[invalid],[valid,empty],[valid,valid],[double]]){
+awaitassertDecompressionStreamRejects('deflate',chunks);
 }
 });
 
-test('DecompressStream gzip emits error on trailing data',async()=>{
+test('DecompressionStream gzip emits TypeError on trailing data',async()=>{
 constvalid=newUint8Array([31,139,8,0,0,0,0,0,0,19,75,4,
 0,67,190,183,232,1,0,0,0]);// gzip('a')
 constempty=newUint8Array(1);
 constinvalid=newUint8Array([...valid, ...empty]);
 constdouble=newUint8Array([...valid, ...valid]);
-for(constchunkof[[invalid],[valid,empty],[valid,valid],[double]]){
-awaitassert.rejects(
-Array.fromAsync(
-newBlob([chunk]).stream().pipeThrough(newDecompressionStream('gzip'))
-),
-{name: 'TypeError'},
-);
+for(constchunksof[[invalid],[valid,empty],[valid,valid],[double]]){
+awaitassertDecompressionStreamRejects('gzip',chunks);
+}
+});
+
+test('DecompressionStream brotli emits TypeError on trailing data',async()=>{
+constvalid=zlib.brotliCompressSync(Buffer.from('a'));
+constempty=newUint8Array(1);
+constinvalid=newUint8Array([...valid, ...empty]);
+constdouble=newUint8Array([...valid, ...valid]);
+for(constchunksof[[invalid],[valid,empty],[valid,valid],[double]]){
+awaitassertDecompressionStreamRejects('brotli',chunks);
 }
 });
+
+test('zlib sync decompression honors rejectGarbageAfterEnd',()=>{
+constvalid=newUint8Array([31,139,8,0,0,0,0,0,0,19,75,4,
+0,67,190,183,232,1,0,0,0]);// gzip('a')
+constdouble=newUint8Array([...valid, ...valid]);
+
+assert.deepStrictEqual(zlib.gunzipSync(double),Buffer.from('aa'));
+assert.throws(
+()=>zlib.gunzipSync(double,{rejectGarbageAfterEnd: true}),
+{code: 'ERR_TRAILING_JUNK_AFTER_STREAM_END',name: 'TypeError'},
+);
+
+constbrotli=zlib.brotliCompressSync(Buffer.from('a'));
+constbrotliDouble=Buffer.concat([brotli,brotli]);
+
+assert.deepStrictEqual(zlib.brotliDecompressSync(brotliDouble),Buffer.from('a'));
+assert.throws(
+()=>zlib.brotliDecompressSync(brotliDouble,{rejectGarbageAfterEnd: true}),
+{code: 'ERR_TRAILING_JUNK_AFTER_STREAM_END',name: 'TypeError'},
+);
+});