From 77ad658635fa9d0907416f347432dbf7b2bf3d5a Mon Sep 17 00:00:00 2001 From: Alexey Zhuchkov Date: Sat, 26 Sep 2026 11:54:43 +0300 Subject: [PATCH] [lib0/decoding] tests for readUint8Array decoder boundaries readUint8Array slices decoder.arr.buffer -- the whole underlying ArrayBuffer -- and never compares `len` against decoder.arr.length. When decoder.arr is a view into a larger buffer (every small Node Buffer is, since allocUnsafe/from/concat serve from a shared 64 KiB slab), a length taken from untrusted input returns bytes that were never part of the message: adjacent process memory, i.e. other connections' data in a server that decodes frames from multiple peers. readVarUint8Array and readVarString inherit it. readVarUint/readVarInt already bound themselves by decoder.arr.length and throw errorUnexpectedEndOfArray (f3c671c); this applies the same limit to the one length-prefixed reader that was missed. Also replaces an unexpected `RangeError: Invalid typed array length` on the out-of-ArrayBuffer path with lib0's own error, consistent with the module. readTailAsUint8Array passes exactly `decoder.arr.length - decoder.pos`, so it is unaffected by the new guard. --- src/encoding.test.js | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/encoding.test.js b/src/encoding.test.js index 6e5d5db..c0d666e 100644 --- a/src/encoding.test.js +++ b/src/encoding.test.js @@ -869,3 +869,46 @@ export const testTerminatedEncodering = _tc => { t.compare(readBuf1, buf1) t.compare(readBuf2, buf2) } + +/** + * A length prefix may not reach beyond the bytes the decoder was given. + * + * Before the bound, `readUint8Array` sliced `decoder.arr.buffer` -- the whole + * underlying ArrayBuffer -- so an over-declared length returned memory outside + * the message. Every small Node Buffer is a view into a shared 64 KiB slab, so + * in a server decoding frames from untrusted peers those bytes belonged to + * other connections. + * + * Note for anyone editing this test: the backing buffer must be large enough + * that the unbounded read would *succeed*, and filled with valid UTF-8, or the + * Uint8Array constructor / the fatal TextDecoder throws and the test passes for + * the wrong reason. + * + * @param {t.TestCase} _tc + */ +export const testReadUint8ArrayDoesNotReadPastTheView = _tc => { + const backing = new Uint8Array(1024) + backing.fill(0x41) + // a 4-byte "message" carrying 2 bytes of payload, as a view inside `backing` + const message = new Uint8Array(backing.buffer, 8, 4) + message[0] = 0x80 // varUint 512 + message[1] = 0x04 + message[2] = 0x41 + message[3] = 0x41 + + t.fails(() => { decoding.readVarUint8Array(decoding.createDecoder(message)) }) + t.fails(() => { decoding.readVarString(decoding.createDecoder(message)) }) + t.fails(() => { decoding.readUint8Array(decoding.createDecoder(message), 5) }) + t.fails(() => { decoding.readUint8Array(decoding.createDecoder(message), -1) }) + + // exact-fit and short reads still work + const decoder = decoding.createDecoder(message) + const exact = decoding.readUint8Array(decoder, 4) + t.assert(exact.length === 4) + t.assert(exact[0] === 0x80 && exact[3] === 0x41) + + // readTailAsUint8Array passes exactly the remaining length, so it is unaffected + const decoder2 = decoding.createDecoder(message) + decoding.readUint8(decoder2) + t.assert(decoding.readTailAsUint8Array(decoder2).length === 3) +}