Skip to content

Commit 81bf44f

Browse files
committed
fix: copy lazy unpack input so transferred buffers cannot dangle
- Copy the caller Buffer before msgpack_unpack_next when lazy is set - str/bin aliases session-owned bytes, not the transferable backing store - Add a transfer test for str and bin after structuredClone detach
1 parent 89161bd commit 81bf44f

5 files changed

Lines changed: 60 additions & 13 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ as accessors so nested values are not converted until they are read. See `#40`.
2323
lazy value also round-trips because it calls `toJSON`.
2424
- Primitives, incomplete buffers, trailing `bytes_remaining`, and the DoS
2525
limits are unchanged. `__proto__` / `constructor` stay own properties.
26+
- Lazy unpack copies the input before decode so str/bin do not alias the
27+
caller's Buffer. Transferring that Buffer after unpack cannot dangle
28+
later property reads.
2629

2730
## [3.1.0] - 2026-09-19
2831

COVERAGE.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,10 @@ gcovr --root . --filter src/ --exclude deps/ --no-markers --txt-metric branch --
150150
identity, nested `o.c[1]` without reading siblings, `__proto__` /
151151
`constructor` as own properties, oversized headers still throw, incomplete
152152
buffers still return `null`, `toJSON` / `JSON.stringify` / `util.inspect`
153-
match eager unpack, nested BigInt, non-object second args, and toJSON
154-
`this` checks. Lazy OOM / empty-Maybe / ObjectTemplate-failure arms are
155-
marked `GCOVR_EXCL_*`, not deleted.
153+
match eager unpack, nested BigInt, non-object second args, toJSON
154+
`this` checks, and str/bin reads after the caller Buffer is transferred.
155+
Lazy OOM / empty-Maybe / ObjectTemplate-failure / CopyBuffer-failure arms
156+
are marked `GCOVR_EXCL_*`, not deleted.
156157
- `test/cli.test.js` (12 tests) — the exit-1 paths of both CLIs: invalid JSON,
157158
empty stdin, a pack rejection reachable from real JSON, an unparseable byte,
158159
an oversized header, incomplete input both alone and after a good frame, and

README.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,13 @@ successful (or attempted) unpack. Stream uses that to splice leftover data.
8383
`unpack(buf, { lazy: true })` wraps maps as objects with accessor
8484
own-properties and arrays as array-likes with indexed accessors. Nested
8585
values are not converted until they are read, which is useful for large
86-
payloads when only a few keys are needed. `JSON.stringify` and
87-
`util.inspect` materialize via `toJSON` / `inspect.custom`. Lazy arrays are
88-
not real `Array`s (`Array.isArray` is false); `pack()` still round-trips
89-
them because it calls `toJSON`. Primitives unpack eagerly even when `lazy`
90-
is set. `__proto__` and `constructor` keys stay own properties, same as
91-
eager unpack.
86+
payloads when only a few keys are needed. The decoder copies `buf` so later
87+
reads do not depend on the caller's backing store (transfer / detach is
88+
safe). `JSON.stringify` and `util.inspect` materialize via `toJSON` /
89+
`inspect.custom`. Lazy arrays are not real `Array`s (`Array.isArray` is
90+
false); `pack()` still round-trips them because it calls `toJSON`.
91+
Primitives unpack eagerly even when `lazy` is set. `__proto__` and
92+
`constructor` keys stay own properties, same as eager unpack.
9293

9394
### Pack type hints (3.1)
9495

src/msgpack.cc

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -678,10 +678,13 @@ static v8::Local<v8::Value> MsgpackToJs(const msgpack_object* mo) {
678678
}
679679

680680
/*
681-
* Lazy unpack: keep the msgpack zone (and the source Buffer) alive, and wrap
682-
* maps/arrays as JS objects whose values are accessors. Nested containers are
683-
* not converted until a property is read. toJSON / inspect.custom materialize
684-
* through MsgpackToJs so JSON.stringify and util.inspect match eager unpack.
681+
* Lazy unpack: keep the msgpack zone (and a session-owned copy of the source
682+
* bytes) alive, and wrap maps/arrays as JS objects whose values are accessors.
683+
* Nested containers are not converted until a property is read. toJSON /
684+
* inspect.custom materialize through MsgpackToJs so JSON.stringify and
685+
* util.inspect match eager unpack. The copy is required because msgpack-c
686+
* aliases str/bin into the input; a Persistent on the caller's Buffer does
687+
* not survive ArrayBuffer transfer.
685688
*/
686689
class LazySession : public Nan::ObjectWrap {
687690
public:
@@ -1075,6 +1078,26 @@ NAN_METHOD(Unpack) {
10751078
return Nan::ThrowError("Encountered error unpacking buffer");
10761079
}
10771080

1081+
/* Copy before unpack_next so via.str/via.bin alias session-owned bytes.
1082+
* Nan::Persistent on the caller's Buffer does not keep the backing store
1083+
* through structuredClone / postMessage transfer (CWE-416). */
1084+
if (UnpackLazyRequested(info)) {
1085+
/* GCOVR_EXCL_BR_START: node Buffers are smaller than UINT32_MAX. */
1086+
if (len > static_cast<size_t>(UINT32_MAX)) {
1087+
return Nan::ThrowError("Error copying buffer");
1088+
}
1089+
/* GCOVR_EXCL_BR_STOP */
1090+
Nan::MaybeLocal<v8::Object> copied =
1091+
Nan::CopyBuffer(data, static_cast<uint32_t>(len));
1092+
/* GCOVR_EXCL_BR_START: CopyBuffer fails only when V8 is out of memory. */
1093+
if (copied.IsEmpty()) {
1094+
return Nan::ThrowError("Error copying buffer");
1095+
}
1096+
/* GCOVR_EXCL_BR_STOP */
1097+
buf = copied.ToLocalChecked();
1098+
data = node::Buffer::Data(buf);
1099+
}
1100+
10781101
msgpack_unpacked result;
10791102
msgpack_unpacked_init(&result);
10801103
size_t off = 0;

test/lazy.test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,4 +187,23 @@ describe('unpack({ lazy: true })', () => {
187187
assert.equal(99 in o, false);
188188
assert.equal(1 in o, true);
189189
});
190+
191+
it('still reads str and bin after the caller Buffer is transferred', () => {
192+
const packed = msgpack.pack({
193+
s: 'hello-lazy-uaf-marker-ABCDEFGH',
194+
t: 'second-string-XXXXYYYY',
195+
n: 7,
196+
b: Buffer.from('bin-payload-1234')
197+
});
198+
const buf = Buffer.allocUnsafeSlow(packed.length);
199+
packed.copy(buf);
200+
const o = msgpack.unpack(buf, { lazy: true });
201+
structuredClone(buf.buffer, { transfer: [buf.buffer] });
202+
assert.equal(buf.buffer.byteLength, 0);
203+
assert.equal(o.n, 7);
204+
assert.equal(o.s, 'hello-lazy-uaf-marker-ABCDEFGH');
205+
assert.equal(o.t, 'second-string-XXXXYYYY');
206+
assert.equal(Buffer.isBuffer(o.b), true);
207+
assert.equal(o.b.toString(), 'bin-payload-1234');
208+
});
190209
});

0 commit comments

Comments
 (0)