diff --git a/.changeset/cbor-decoder-bounds.md b/.changeset/cbor-decoder-bounds.md new file mode 100644 index 00000000..04d96a25 --- /dev/null +++ b/.changeset/cbor-decoder-bounds.md @@ -0,0 +1,9 @@ +--- +"@evolution-sdk/evolution": patch +--- + +The CBOR decoder now rejects truncated and length-lying input with a `CBORError` instead of reading past the end of the input or allocating for a declared length. `CBOR.fromCBORHex("9a7fffffff")` — an array header declaring 2^31 - 1 items over five bytes — previously aborted the Node process with a heap out-of-memory that `try`/`catch` cannot stop; it now throws immediately. Three checks, all of which well-formed CBOR never hits: + +- Decoding an item at or past the end of the input throws, instead of `data[offset]` reading `undefined` and decoding as the integer 0. +- A definite array or map whose declared length needs more bytes than remain throws before any allocation or looping. +- A 4-byte length is read unsigned, so a length with the top bit set (e.g. `0xffffffff`) no longer reads as a negative number. diff --git a/packages/evolution/src/CBOR.ts b/packages/evolution/src/CBOR.ts index 3bb1e7c7..751333db 100644 --- a/packages/evolution/src/CBOR.ts +++ b/packages/evolution/src/CBOR.ts @@ -1904,6 +1904,12 @@ const bytesReadToByteSize = (bytesRead: number): ByteSize => bytesRead <= 1 ? 0 : bytesRead === 2 ? 1 : bytesRead === 3 ? 2 : 4 const decodeItemAt = (data: Uint8Array, offset: number, options: CodecOptions, track: DecodeTrack): DecodeAtResult => { + // An offset at or past the end means the input is truncated: `data[offset]` + // would be `undefined`, which bit-shifts to major type 0 and silently + // decodes as the integer 0 instead of failing. + if (offset >= data.length) { + throw new CBORError({ message: `Insufficient data: expected a CBOR item at offset ${offset}` }) + } const firstByte = data[offset] const majorType = (firstByte >> 5) & 0x07 let result: DecodeAtResult @@ -2017,7 +2023,9 @@ const decodeLengthAt = (data: Uint8Array, offset: number): { length: number; byt } else if (additionalInfo === 26) { if (data.length < offset + 5) throw new CBORError({ message: "Insufficient data for 4-byte length" }) return { - length: (data[offset + 1] << 24) | (data[offset + 2] << 16) | (data[offset + 3] << 8) | data[offset + 4], + // `>>> 0` keeps the result unsigned: without it a length with the top + // bit set (e.g. 0xffffffff) reads as a negative number. + length: ((data[offset + 1] << 24) | (data[offset + 2] << 16) | (data[offset + 3] << 8) | data[offset + 4]) >>> 0, bytesRead: 5 } } @@ -2143,6 +2151,12 @@ const decodeArrayAt = (data: Uint8Array, offset: number, options: CodecOptions, } else { const { bytesRead, length } = decodeLengthAt(data, offset) let cur = offset + bytesRead + // Every item takes at least one byte, so a declared length larger than + // the bytes left is impossible. Reject it before `new Array(length)` + // turns a lying header into a huge allocation. + if (length > data.length - cur) { + throw new CBORError({ message: `Insufficient data: array declares ${length} items with ${data.length - cur} bytes left` }) + } const arr: Array = new Array(length) if (track === "none") { for (let i = 0; i < length; i++) { @@ -2197,6 +2211,12 @@ const decodeMapAt = (data: Uint8Array, offset: number, options: CodecOptions, tr } else { const { bytesRead, length } = decodeLengthAt(data, offset) let cur = offset + bytesRead + // Every key and value takes at least one byte, so a declared pair count + // needing more than the bytes left is impossible; reject it up front + // instead of looping (or allocating) on a lying header. + if (length * 2 > data.length - cur) { + throw new CBORError({ message: `Insufficient data: map declares ${length} pairs with ${data.length - cur} bytes left` }) + } const map = isObj ? ({} as Record) : new Map() if (track === "none") { for (let i = 0; i < length; i++) { diff --git a/packages/evolution/test/CBOR.test.ts b/packages/evolution/test/CBOR.test.ts index dd9f0907..da0f6857 100644 --- a/packages/evolution/test/CBOR.test.ts +++ b/packages/evolution/test/CBOR.test.ts @@ -417,6 +417,66 @@ describe("CBOR Implementation Tests", () => { }) }) + describe("CBOR Decoder Bounds (issue #661)", () => { + it("should reject a declared array length larger than the input instead of allocating for it", () => { + // Array of 2^31 - 1 items with no items present. Before the fix this + // aborted the process with a heap out-of-memory that try/catch cannot stop. + expect(() => { + CBOR.fromCBORHex("9a7fffffff") + }).toThrowError(CBOR.CBORError) + + expect(() => { + CBOR.fromCBORHexWithFormat("9a7fffffff") + }).toThrowError(CBOR.CBORError) + }) + + it("should reject a declared map length larger than the input", () => { + expect(() => { + CBOR.fromCBORHex("ba7fffffff") // 2^31 - 1 pairs, none present + }).toThrowError(CBOR.CBORError) + + // 0xffffffff as a 4-byte length used to read as -1 through signed + // 32-bit shifts and slip past the loop entirely. + expect(() => { + CBOR.fromCBORHex("baffffffff") + }).toThrowError(CBOR.CBORError) + }) + + it("should reject definite collections whose items run past the end", () => { + expect(() => { + CBOR.fromCBORHex("8300") // declares 3 items, holds 1 + }).toThrowError(CBOR.CBORError) + + expect(() => { + CBOR.fromCBORHex("81") // declares 1 item, holds none + }).toThrowError(CBOR.CBORError) + + expect(() => { + CBOR.fromCBORHex("a100") // declares 1 pair, holds only a key + }).toThrowError(CBOR.CBORError) + }) + + it("should reject an item that starts at the end of the input", () => { + // Tag 0 with no tagged item after it: the nested decode begins exactly + // at the end of the input, which used to decode as the integer 0. + expect(() => { + CBOR.fromCBORHex("c0") + }).toThrowError(CBOR.CBORError) + + expect(() => { + CBOR.fromCBORHex("81c0") // same, nested inside a 1-item array + }).toThrowError(CBOR.CBORError) + }) + + it("should still decode well-formed collections at the boundary", () => { + expect(CBOR.fromCBORHex("83010203")).toEqual([1n, 2n, 3n]) + expect(CBOR.fromCBORHex("a10102")).toEqual(new Map([[1n, 2n]])) + // 24 items announced with a 1-byte length header, all present + const items = Array.from({ length: 24 }, () => "00").join("") + expect(CBOR.fromCBORHex(`9818${items}`)).toEqual(Array.from({ length: 24 }, () => 0n)) + }) + }) + describe("RFC 8949 CBOR Test Vectors", () => { // Test vectors from https://github.com/cbor/test-vectors/blob/master/appendix_a.json // These are the official CBOR specification test cases from RFC 8949 Appendix A