zlib: reject truncated zstd input

Treat an unfinished Zstd frame as an unexpected end of file when the
stream is finalized with ZSTD_e_end.

Avoid reporting an error while the output buffer still needs to be
drained or when an empty final write follows a completed frame. Preserve
partial decompression when ZSTD_e_flush is used.

Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>
PR-URL: #64593
Fixes: #64592
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ryuhei Shima <shimaryuhei@gmail.com>

Author: Archkon

src/node_zlib.cc

@@ -358,6 +358,7 @@ class ZstdDecompressContext final : public ZstdContext {
   // Streaming-related, should be available for all compression libraries:
   void Close();
   void DoThreadPoolWork();
+  CompressionError GetErrorInfo() const;
   CompressionError ResetStream();
 
   // Zstd specific:
@@ -375,6 +376,7 @@ class ZstdDecompressContext final : public ZstdContext {
 
  private:
   DeleteFnPtr<ZSTD_DCtx, ZstdDecompressContext::FreeZstd> dctx_;
+  bool frame_complete_ = false;
 };
 
 class CompressionStreamMemoryOwner {
@@ -1717,6 +1719,8 @@ void ZstdDecompressContext::Close() {
 
 CompressionError ZstdDecompressContext::Init(uint64_t pledged_src_size,
                                              std::string_view dictionary) {
+  frame_complete_ = false;
+
 #ifdef NODE_BUNDLED_ZSTD
   ZSTD_customMem custom_mem = {
       CompressionStreamMemoryOwner::AllocForBrotli,
@@ -1752,12 +1756,37 @@ CompressionError ZstdDecompressContext::ResetStream() {
 }
 
 void ZstdDecompressContext::DoThreadPoolWork() {
+  // The JavaScript processing loop retries with an empty input buffer when the
+  // previous call filled the output buffer. Avoid interpreting that retry as
+  // the beginning of a new, incomplete frame.
+  if (frame_complete_ && input_.size == 0) {
+    return;
+  }
+
   size_t const ret = ZSTD_decompressStream(dctx_.get(), &output_, &input_);
   if (ZSTD_isError(ret)) {
+    frame_complete_ = false;
     error_ = ZSTD_getErrorCode(ret);
     error_code_string_ = ZstdStrerror(error_);
     error_string_ = ZSTD_getErrorString(error_);
+  } else {
+    frame_complete_ = ret == 0;
+  }
+}
+
+CompressionError ZstdDecompressContext::GetErrorInfo() const {
+  CompressionError error = ZstdContext::GetErrorInfo();
+  if (error.IsError()) {
+    return error;
   }
+
+  if (flush_ == ZSTD_e_end && !frame_complete_ && input_.pos == input_.size &&
+      output_.pos < output_.size) {
+    return CompressionError(
+        "unexpected end of file", "Z_BUF_ERROR", Z_BUF_ERROR);
+  }
+
+  return {};
 }
 
 template <typename Stream>

test/parallel/test-zlib-truncated.js

@@ -22,6 +22,12 @@ const errMessage = /unexpected end of file/;
   { comp: 'gzip', decomp: 'unzip', decompSync: 'unzipSync' },
   { comp: 'deflate', decomp: 'inflate', decompSync: 'inflateSync' },
   { comp: 'deflateRaw', decomp: 'inflateRaw', decompSync: 'inflateRawSync' },
+  {
+    comp: 'zstdCompress',
+    decomp: 'zstdDecompress',
+    decompSync: 'zstdDecompressSync',
+    partialFlush: zlib.constants.ZSTD_e_flush,
+  },
 ].forEach(function(methods) {
   zlib[methods.comp](inputString, common.mustSucceed((compressed) => {
     const truncated = compressed.slice(0, compressed.length / 2);
@@ -46,16 +52,59 @@ const errMessage = /unexpected end of file/;
       assert.match(err.message, errMessage);
     }));
 
-    const syncFlushOpt = { finishFlush: zlib.constants.Z_SYNC_FLUSH };
+    const partialFlushOpt = {
+      finishFlush: methods.partialFlush ?? zlib.constants.Z_SYNC_FLUSH,
+    };
 
-    // Sync truncated input test, finishFlush = Z_SYNC_FLUSH
-    const result = toUTF8(zlib[methods.decompSync](truncated, syncFlushOpt));
+    // Sync truncated input test with a non-finalizing finish flush.
+    const result = toUTF8(zlib[methods.decompSync](truncated, partialFlushOpt));
     assert.strictEqual(result, inputString.slice(0, result.length));
 
-    // Async truncated input test, finishFlush = Z_SYNC_FLUSH
-    zlib[methods.decomp](truncated, syncFlushOpt, common.mustSucceed((decompressed) => {
+    // Async truncated input test with a non-finalizing finish flush.
+    zlib[methods.decomp](truncated, partialFlushOpt, common.mustSucceed((decompressed) => {
       const result = toUTF8(decompressed);
       assert.strictEqual(result, inputString.slice(0, result.length));
     }));
   }));
 });
+
+// A non-zero return from ZSTD_decompressStream() can also mean that the
+// output buffer is full. Make sure that is drained before treating the return
+// value as truncated input.
+{
+  const input = Buffer.alloc(zlib.constants.Z_DEFAULT_CHUNK * 2, 0x61);
+  const compressed = zlib.zstdCompressSync(input);
+  const decompressed = zlib.zstdDecompressSync(compressed, {
+    chunkSize: zlib.constants.Z_MIN_CHUNK,
+  });
+  assert.deepStrictEqual(decompressed, input);
+}
+
+// Ending a stream after a previous write completed a frame must not be
+// mistaken for an empty, truncated frame.
+{
+  const input = Buffer.from(inputString);
+  const compressed = zlib.zstdCompressSync(input);
+  const decompressor = zlib.createZstdDecompress();
+  const output = [];
+
+  decompressor.on('data', (chunk) => output.push(chunk));
+  decompressor.on('end', common.mustCall(() => {
+    assert.deepStrictEqual(Buffer.concat(output), input);
+  }));
+  decompressor.write(compressed, common.mustCall(() => decompressor.end()));
+}
+
+// Conversely, ending after a previous write supplied only part of a frame
+// must report that the frame is incomplete.
+{
+  const compressed = zlib.zstdCompressSync(inputString);
+  const truncated = compressed.subarray(0, compressed.length / 2);
+  const decompressor = zlib.createZstdDecompress();
+
+  decompressor.on('error', common.mustCall((error) => {
+    assert.match(error.message, errMessage);
+  }));
+  decompressor.write(truncated, common.mustCall(() => decompressor.end()));
+  decompressor.resume();
+}