fix(node_binding): harden the compressed-binding packer per review

- unpack() fails closed instead of crashing: bound compressed/uncompressed
  lengths before narrowing to Number (no precision loss), reject a compressed
  length that overruns the section, and catch zstd decompress errors.
- drop the unused fs import from the packer.
- bench: resolve the script dir with fileURLToPath (correct on Windows) instead
  of a URL pathname.
- fix header comment grammar (unwraps / creator-side).
- add unit tests for the overrun and non-zstd-payload paths.

Author: jdalton

crates/node_binding/scripts/bench-compressed-binding.mjs

@@ -16,6 +16,7 @@ import { createRequire } from 'node:module'
 import fs from 'node:fs'
 import path from 'node:path'
 import process from 'node:process'
+import { fileURLToPath } from 'node:url'
 import zlib from 'node:zlib'
 
 const require = createRequire(import.meta.url)
@@ -44,7 +45,7 @@ function resolveBindingNode(triple) {
   try {
     return require.resolve(`@rspack/binding-${triple}/${file}`)
   } catch {}
-  const roots = [process.cwd(), path.dirname(new URL(import.meta.url).pathname)]
+  const roots = [process.cwd(), path.dirname(fileURLToPath(import.meta.url))]
   for (const root of roots) {
     let dir = root
     for (let i = 0; i < 8; i += 1) {

crates/node_binding/scripts/pack-compressed-binding.mjs

@@ -1,17 +1,16 @@
 // Pack a built `rspack.<platform>.node` into the "pressed-data" payload used by
 // the compressed-binding loader: zstd-compressed addon + SHA-512 integrity, in
-// the same layout the `decmpfs` crate (and bin-infra) already unwrap. This is
-// the create-side reference; the build step injects the payload into the
+// the same layout the `decmpfs` crate (and bin-infra) already unwraps. This is
+// the creator-side reference; the build step injects the payload into the
 // binary's PRESSED_DATA section and the Rust loader unwraps it on first load.
 //
 // Format: [magic 32B "__SMOL_PRESSED_DATA_MAGIC_MARKER"]
 //   [compressed u64 LE][uncompressed u64 LE][cache key 16B][platform 3B]
 //   [SHA-512 of the zstd payload 64B][has_config 1B][zstd frame]
 //
 // No @napi-rs/cli, no legacy-Node fallback — modern node:zlib/node:crypto only.
-import zlib from 'node:zlib'
 import crypto from 'node:crypto'
-import fs from 'node:fs'
+import zlib from 'node:zlib'
 
 const MAGIC = Buffer.from('__SMOL_PRESSED_DATA_MAGIC_MARKER') // 32B
 const CACHE_KEY_LEN = 16
@@ -43,15 +42,17 @@ export function pack(raw, options) {
   return Buffer.concat([head, payload])
 }
 
+const MAX_LEN = 512 * 1024 * 1024 // reject implausible sizes; keeps sizes in Number's safe range
+
 export function unpack(section) {
   const min = MAGIC.length + 8 + 8 + CACHE_KEY_LEN + PLATFORM_LEN + HASH_LEN + 1
   if (section.length < min || !section.subarray(0, MAGIC.length).equals(MAGIC)) {
     return null
   }
   let o = MAGIC.length
-  const clen = Number(section.readBigUInt64LE(o))
+  const clenBig = section.readBigUInt64LE(o)
   o += 8
-  const ulen = Number(section.readBigUInt64LE(o))
+  const ulenBig = section.readBigUInt64LE(o)
   o += 8
   o += CACHE_KEY_LEN + PLATFORM_LEN
   const hash = section.subarray(o, o + HASH_LEN)
@@ -60,13 +61,33 @@ export function unpack(section) {
     o += 1192 // config block
   }
   o += 1
-  if (!clen || !ulen || ulen > 512 * 1024 * 1024) {
+  // Bound both lengths BEFORE narrowing to Number, so a huge u64 can't lose
+  // precision or drive an out-of-range read.
+  if (
+    clenBig <= 0n ||
+    ulenBig <= 0n ||
+    clenBig > BigInt(MAX_LEN) ||
+    ulenBig > BigInt(MAX_LEN)
+  ) {
+    return null
+  }
+  const clen = Number(clenBig)
+  const ulen = Number(ulenBig)
+  // The declared payload must actually fit in what's left of the section.
+  if (o + clen > section.length) {
     return null
   }
   const payload = section.subarray(o, o + clen)
   if (!crypto.createHash('sha512').update(payload).digest().equals(hash)) {
     return null
   }
-  const raw = zlib.zstdDecompressSync(payload)
+  // A corrupt payload that still matched the hash (or a non-zstd frame) must
+  // fail closed, never crash the loader.
+  let raw
+  try {
+    raw = zlib.zstdDecompressSync(payload)
+  } catch {
+    return null
+  }
   return raw.length === ulen ? raw : null
 }

crates/node_binding/scripts/pack-compressed-binding.test.mjs

@@ -62,3 +62,31 @@ test('unpack rejects an implausibly large uncompressed size', () => {
   packed.writeBigUInt64LE(BigInt(600 * 1024 * 1024), MAGIC.length + 8)
   assert.equal(unpack(packed), null)
 })
+
+test('unpack rejects a compressed length that overruns the buffer', () => {
+  const packed = pack(Buffer.from('rspack payload'))
+  // clen (offset magic+0) claims more bytes than the section actually holds,
+  // but stays under the 512 MiB cap — the fit check must catch it.
+  packed.writeBigUInt64LE(BigInt(packed.length + 1024), MAGIC.length)
+  assert.equal(unpack(packed), null)
+})
+
+test('unpack fails closed (no throw) on a non-zstd payload with a valid hash', () => {
+  // Hand-build a well-formed header whose stored SHA-512 matches the payload,
+  // but the payload is not a zstd frame — decompress throws, unpack returns null.
+  const junk = Buffer.from('this is not a zstd frame', 'utf8')
+  const head = Buffer.alloc(HEAD_LEN)
+  let o = 0
+  MAGIC.copy(head, o)
+  o += MAGIC.length
+  head.writeBigUInt64LE(BigInt(junk.length), o) // clen
+  o += 8
+  head.writeBigUInt64LE(64n, o) // ulen (plausible, under cap)
+  o += 8
+  o += 16 + 3 // cache key + platform
+  crypto.createHash('sha512').update(junk).digest().copy(head, o) // matching hash
+  o += 64
+  head[o] = 0 // has_config
+  const section = Buffer.concat([head, junk])
+  assert.equal(unpack(section), null)
+})