diff --git a/packages/archive-codec/src/cfb/ole-package.test.ts b/packages/archive-codec/src/cfb/ole-package.test.ts index d88bdc36f6..65c7c38b50 100644 --- a/packages/archive-codec/src/cfb/ole-package.test.ts +++ b/packages/archive-codec/src/cfb/ole-package.test.ts @@ -79,19 +79,58 @@ describe("readOlePackage", () => { it("throws OlePackageFormatError when a string never terminates", () => { const unterminated = enc("\x02\x00Book1.xlsx"); + let caught: unknown; try { readOlePackage(unterminated); - throw new Error("expected readOlePackage to throw OlePackageFormatError"); } catch (error) { - expect(error).toBeInstanceOf(OlePackageFormatError); + caught = error; } + expect(caught).toBeInstanceOf(OlePackageFormatError); + expect((caught as Error).name).toBe("OlePackageFormatError"); + expect((caught as Error).message).toBe( + "Package stream ends inside its label string with no terminator", + ); + }); + + it("names the field that never terminates, when it is the source path rather than the label", () => { + const unterminated = enc("\x02\x00a\0Book1.xlsx"); + expect(() => readOlePackage(unterminated)).toThrow( + "Package stream ends inside its source path string with no terminator", + ); + }); + + it("names the field that never terminates, when it is the temp path rather than the label or source path", () => { + const unterminated = new Uint8Array([ + ...enc("\x02\x00a\0b\0"), + ...new Uint8Array(8), // the 8 opaque bytes between sourcePath and tempPath + ...enc("Book1.xlsx"), + ]); + expect(() => readOlePackage(unterminated)).toThrow( + "Package stream ends inside its temp path string with no terminator", + ); + }); + + it("throws OlePackageFormatError when fewer than 4 bytes remain for the packaged file's own size field", () => { + // The three strings and the 8 opaque bytes are all present and well-formed; only the trailing size field itself is truncated. + const bytes = new Uint8Array([ + ...enc("\x02\x00a\0b\0"), + ...new Uint8Array(8), // the 8 opaque bytes + ...enc("c\0"), + 0x00, + 0x00, + ]); // 2 bytes where the 4-byte size field belongs + expect(() => readOlePackage(bytes)).toThrow( + "Package stream ends before its packaged file size field", + ); }); it("throws OlePackageFormatError when the declared file size exceeds the remaining bytes", () => { const wrapped = packageStream("a", "b", "c", enc("payload")); const view = new DataView(wrapped.buffer); view.setUint32(wrapped.length - "payload".length - 4, 0x00ffffff, true); - expect(() => readOlePackage(wrapped)).toThrow(OlePackageFormatError); + expect(() => readOlePackage(wrapped)).toThrow( + "Package stream declares 16777215 packaged-file bytes but holds only 7", + ); }); it("throws OlePackageFormatError for input too short to hold even the fixed fields", () => { @@ -102,6 +141,16 @@ describe("readOlePackage", () => { }); describe("writeOlePackage", () => { + it("writes the header word as 0x0002 in little-endian order", () => { + const built = writeOlePackage({ + label: "", + sourcePath: "", + tempPath: "", + fileBytes: new Uint8Array(0), + }); + expect([...built.subarray(0, 2)]).toEqual([0x02, 0x00]); + }); + it("round-trips through readOlePackage", () => { const fileBytes = enc("the real embedded file"); const built = writeOlePackage({ @@ -133,6 +182,17 @@ describe("writeOlePackage", () => { expect(parsed.fileBytes).toEqual(new Uint8Array(0)); }); + it("accepts U+007F (DEL), the highest code point this field's ASCII check allows", () => { + expect(() => + writeOlePackage({ + label: "a\u007fb", + sourcePath: "", + tempPath: "", + fileBytes: new Uint8Array(0), + }), + ).not.toThrow(); + }); + it("throws OlePackageWriteError when label contains a non-ASCII character", () => { expect(() => writeOlePackage({ @@ -141,7 +201,9 @@ describe("writeOlePackage", () => { tempPath: "", fileBytes: new Uint8Array(0), }), - ).toThrow(OlePackageWriteError); + ).toThrow( + "Package stream's label contains a character (U+00e9) outside ASCII; encoding it to an arbitrary windows-1252 byte would need a full codepage table this package does not carry", + ); }); it("throws OlePackageWriteError when sourcePath or tempPath contains a non-ASCII character", () => { @@ -152,7 +214,7 @@ describe("writeOlePackage", () => { tempPath: "", fileBytes: new Uint8Array(0), }), - ).toThrow(OlePackageWriteError); + ).toThrow("Package stream's source path contains a character"); expect(() => writeOlePackage({ label: "a", @@ -160,19 +222,27 @@ describe("writeOlePackage", () => { tempPath: "C:\\café\\a.docx", fileBytes: new Uint8Array(0), }), - ).toThrow(OlePackageWriteError); + ).toThrow("Package stream's temp path contains a character"); }); // A NUL byte is itself ASCII (U+0000, well under 0x7f), so the non-ASCII check above cannot catch it -- but this field's own encoding is null-terminated, so an embedded NUL would silently truncate the field and mis-frame every field written after it, exactly the round-trip guarantee this function's own doc comment states. it("throws OlePackageWriteError when label contains an embedded NUL byte", () => { - expect(() => + let caught: unknown; + try { writeOlePackage({ label: "a\u0000b", sourcePath: "", tempPath: "", fileBytes: new Uint8Array(0), - }), - ).toThrow(OlePackageWriteError); + }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(OlePackageWriteError); + expect((caught as Error).name).toBe("OlePackageWriteError"); + expect((caught as Error).message).toBe( + "Package stream's label contains an embedded NUL byte, which this field's own null-terminated encoding cannot carry: it would silently truncate the field and mis-frame every field written after it", + ); }); it("throws OlePackageWriteError when sourcePath or tempPath contains an embedded NUL byte", () => { @@ -183,7 +253,7 @@ describe("writeOlePackage", () => { tempPath: "", fileBytes: new Uint8Array(0), }), - ).toThrow(OlePackageWriteError); + ).toThrow("Package stream's source path contains an embedded NUL byte"); expect(() => writeOlePackage({ label: "a", @@ -191,6 +261,6 @@ describe("writeOlePackage", () => { tempPath: "C:\\a\u0000b.docx", fileBytes: new Uint8Array(0), }), - ).toThrow(OlePackageWriteError); + ).toThrow("Package stream's temp path contains an embedded NUL byte"); }); }); diff --git a/packages/archive-codec/src/cfb/ole-package.ts b/packages/archive-codec/src/cfb/ole-package.ts index 559cc917fd..d3561bd091 100644 --- a/packages/archive-codec/src/cfb/ole-package.ts +++ b/packages/archive-codec/src/cfb/ole-package.ts @@ -25,11 +25,9 @@ function readZeroTerminated( offset: number, fieldName: string, ): { readonly value: string; readonly next: number } { - let end = offset; - while (end < bytes.length && bytes[end] !== 0) { - end++; - } - if (end >= bytes.length) { + // indexOf, not a hand-rolled scanning loop with its own bounds check: it already reports "not found" as a single -1 sentinel, so there is exactly one place (below) that decides whether the terminator was found, not two redundant bounds checks that could disagree. + const end = bytes.indexOf(0, offset); + if (end === -1) { throw new OlePackageFormatError( `Package stream ends inside its ${fieldName} string with no terminator`, ); @@ -85,8 +83,10 @@ function asciiZeroTerminated( fieldName: string, ): Uint8Array { const bytes = new Uint8Array(value.length + 1); // +1 for the terminator, already zero from the Uint8Array's own zero-fill - for (let index = 0; index < value.length; index++) { - const code = value.charCodeAt(index); + const view = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength); + // Walks value.split("") rather than a `for` loop bound by value.length: the allocation's own reserved terminator byte sits right after the last character, so a loop bound one iteration too long would write its extra byte there -- a genuinely equivalent mutant, since that byte is already zero and no test could ever observe the difference. split("") has no comparison bound to mismeasure in the first place. + value.split("").forEach((char, index) => { + const code = char.charCodeAt(0); if (code === 0) { throw new OlePackageWriteError( `Package stream's ${fieldName} contains an embedded NUL byte, which this field's own null-terminated encoding cannot carry: it would silently truncate the field and mis-frame every field written after it`, @@ -97,8 +97,8 @@ function asciiZeroTerminated( `Package stream's ${fieldName} contains a character (U+${code.toString(16).padStart(4, "0")}) outside ASCII; encoding it to an arbitrary windows-1252 byte would need a full codepage table this package does not carry`, ); } - bytes[index] = code; - } + view.setUint8(index, code); + }); return bytes; } diff --git a/packages/archive-codec/src/cfb/read.test.ts b/packages/archive-codec/src/cfb/read.test.ts index 2b9eb15101..da7171600b 100644 --- a/packages/archive-codec/src/cfb/read.test.ts +++ b/packages/archive-codec/src/cfb/read.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { compoundFile } from "../test-support/cfb"; import { CompoundFileFormatError, readCompoundFile } from "./read"; +import { writeCompoundFile } from "./write"; // Coverage for the bounded [MS-CFB] reader (src/cfb/read.ts): header/sector-size parsing, DIFAT/FAT chain walking, the directory entry tree, stream extraction from both the FAT and the mini stream, and the guards. Fixtures come from src/test-support/cfb.ts -- a hand-built minimal compound-file writer whose construction is documented there -- because the reader under test consumes actual compound-file bytes (a hand-built in-memory model would skip the parse entirely). @@ -87,6 +88,167 @@ describe("readCompoundFile", () => { it("returns an empty listing for a compound file with no streams", () => { expect(readCompoundFile(compoundFile([]))).toEqual([]); }); + + it("writes ENDOFCHAIN as the root entry's own starting sector when there is no mini stream at all", () => { + // header (512) + one FAT sector (512) + the root entry (id 0) at the directory sector's own start. + const bytes = compoundFile([]); + expect(new DataView(bytes.buffer).getUint32(512 * 2 + 0x74, true)).toBe( + 0xfffffffe, + ); + }); + + it("extracts several mini-resident streams from the same mini stream, each at its own sequential mini sector", () => { + // Four same-length names (so length-first sibling ordering, mirrored from [MS-CFB] 2.6.4, leaves them in plain alphabetical/insertion order) with individually distinguishable mini-sector counts: 1, 2, 1, and 3 mini sectors (64 bytes each). + const streams = readCompoundFile( + compoundFile([ + { path: "Aaa", bytes: enc("a".repeat(30)) }, // ceil(30/64) = 1 + { path: "Bbb", bytes: enc("b".repeat(100)) }, // ceil(100/64) = 2 + { path: "Ccc", bytes: enc("c".repeat(10)) }, // ceil(10/64) = 1 + { path: "Ddd", bytes: enc("d".repeat(150)) }, // ceil(150/64) = 3 + ]), + ); + expect(streams.map((s) => s.path)).toEqual(["Aaa", "Bbb", "Ccc", "Ddd"]); + expect(streams.find((s) => s.path === "Aaa")?.bytes).toEqual( + enc("a".repeat(30)), + ); + expect(streams.find((s) => s.path === "Bbb")?.bytes).toEqual( + enc("b".repeat(100)), + ); + expect(streams.find((s) => s.path === "Ccc")?.bytes).toEqual( + enc("c".repeat(10)), + ); + expect(streams.find((s) => s.path === "Ddd")?.bytes).toEqual( + enc("d".repeat(150)), + ); + }); + + it("extracts several FAT-resident streams, each occupying its own run of whole sectors", () => { + const streams = readCompoundFile( + compoundFile([ + { path: "One", bytes: enc("1".repeat(5000)) }, + { path: "Two", bytes: enc("2".repeat(6000)) }, + { path: "Three", bytes: enc("3".repeat(4200)) }, + ]), + ); + expect(streams.find((s) => s.path === "One")?.bytes).toEqual( + enc("1".repeat(5000)), + ); + expect(streams.find((s) => s.path === "Two")?.bytes).toEqual( + enc("2".repeat(6000)), + ); + expect(streams.find((s) => s.path === "Three")?.bytes).toEqual( + enc("3".repeat(4200)), + ); + }); + + it("extracts a storage with several sibling children, not just one", () => { + const streams = readCompoundFile( + compoundFile([ + { path: "Pool/First", bytes: enc("1") }, + { path: "Pool/Second", bytes: enc("2") }, + { path: "Pool/Third", bytes: enc("3") }, + ]), + ); + expect(streams.map((s) => s.path).sort()).toEqual([ + "Pool/First", + "Pool/Second", + "Pool/Third", + ]); + }); + + it("reads every stream of a file needing more than one 512-byte directory sector (more than 4 entries)", () => { + // 4 entries per 512-byte directory sector ([MS-CFB] 2.6.1's 128-byte entry): 10 streams plus the root need 3 directory sectors. + const inputs = Array.from({ length: 10 }, (_unused, index) => ({ + path: `Stream${index}`, + bytes: enc(`payload ${index}`), + })); + const streams = readCompoundFile(compoundFile(inputs)); + expect(streams).toHaveLength(10); + for (const input of inputs) { + expect(streams.find((s) => s.path === input.path)?.bytes).toEqual( + input.bytes, + ); + } + }); + + it("reads a file large enough to need more than one FAT sector", () => { + // A 512-byte-sector FAT sector maps 128 sectors (64 KiB); a 300 KiB stream forces the fixed-point FAT-sector-count loop to grow past 1 and reach a genuine fixed point. + const payload = new Uint8Array(300 * 1024); + for (let i = 0; i < payload.length; i++) { + payload[i] = (i * 13 + 5) & 0xff; + } + const bytes = compoundFile([{ path: "Big", bytes: payload }]); + expect(new DataView(bytes.buffer).getUint32(0x2c, true)).toBeGreaterThan(1); + const streams = readCompoundFile(bytes); + expect(streams[0]?.bytes).toEqual(payload); + }, 20000); // v8 coverage instrumentation (CI's own _test:coverage task, and every Stryker mutant run) multiplies this test's real cost far past the default 5000ms budget: a 300 KiB byte-fill loop plus a full round trip is measured well under a second uninstrumented, but has been observed to exceed 5s on a loaded GitHub runner under coverage. A generous fixed timeout, not a smaller payload, keeps the fixture large enough to force the fixed-point loop past 1 while removing the flake. + + it("needs a second mini FAT sector once the mini stream passes 128 mini sectors", () => { + // Each stream's own byte content is distinct (filled with its own index), not uniformly zero: a mini-FAT sector physically misplaced during the write would corrupt whichever OTHER stream's data actually occupies that sector, and only content that differs per stream can make that corruption visible -- an all-zero payload would still read back as all zero even after such a misplacement. + const miniSectorsNeeded = 129; + const inputs = Array.from({ length: miniSectorsNeeded }, (_unused, i) => ({ + path: `M${i}`, + bytes: new Uint8Array(64).fill(i % 256), // exactly one mini sector each + })); + const bytes = compoundFile(inputs); + expect(new DataView(bytes.buffer).getUint32(0x40, true)).toBe(2); + const streams = readCompoundFile(bytes); + expect(streams).toHaveLength(miniSectorsNeeded); + for (let i = 0; i < miniSectorsNeeded; i++) { + expect(streams.find((s) => s.path === `M${i}`)?.bytes).toEqual( + new Uint8Array(64).fill(i % 256), + ); + } + }); + + it("writes 0 as the directory-sector count for a version 3 file, and the real count for version 4", () => { + const inputs = Array.from({ length: 10 }, (_unused, index) => ({ + path: `Stream${index}`, + bytes: enc("x"), + })); + const v3 = compoundFile(inputs, { majorVersion: 3 }); + const v4 = compoundFile(inputs, { majorVersion: 4 }); + expect(new DataView(v3.buffer).getUint32(0x28, true)).toBe(0); + expect(new DataView(v4.buffer).getUint32(0x28, true)).toBeGreaterThan(0); + }); +}); + +describe("compoundFile input validation", () => { + it("rejects a storage or stream name that is empty", () => { + expect(() => compoundFile([{ path: "", bytes: enc("x") }])).toThrow(); + }); + + it("rejects a storage or stream name longer than 31 characters", () => { + expect(() => + compoundFile([{ path: "N".repeat(32), bytes: enc("x") }]), + ).toThrow(/at most 31 characters/); + expect(() => + compoundFile([{ path: "N".repeat(31), bytes: enc("x") }]), + ).not.toThrow(); + }); + + it("rejects a storage or stream name holding a non-ASCII byte", () => { + expect(() => compoundFile([{ path: "café", bytes: enc("x") }])).toThrow( + /non-empty ASCII/, + ); + }); + + it("rejects an empty path segment", () => { + for (const path of ["/Leading", "Trailing/", "Double//Segment"]) { + expect(() => compoundFile([{ path, bytes: enc("x") }])).toThrow( + /no empty segments/, + ); + } + }); + + it("rejects the same path supplied twice", () => { + expect(() => + compoundFile([ + { path: "Dup", bytes: enc("1") }, + { path: "Dup", bytes: enc("2") }, + ]), + ).toThrow(/used twice/); + }); }); describe("readCompoundFile malformed-input handling", () => { @@ -102,31 +264,215 @@ describe("readCompoundFile malformed-input handling", () => { } }; - it("throws for bytes without the compound-file signature", () => { - expectFormatError(enc("not a compound file at all")); + // Layout constants for the single-stream, single-FAT-sector fixtures below: header (512) + one FAT sector (512) puts the directory at byte 1024, entry 0 (root) at 1024, entry 1 (the one stream) at 1024 + 128. + const HEADER_BYTES = 512; + const FAT_SECTOR_BYTES = 512; + const DIRECTORY_START = HEADER_BYTES + FAT_SECTOR_BYTES; + const entryOffset = (id: number): number => DIRECTORY_START + id * 128; + + it("names its own error class CompoundFileFormatError, not merely an instance of it", () => { + let caught: unknown; + try { + readCompoundFile(enc("not a compound file at all")); + } catch (error) { + caught = error; + } + expect((caught as Error).name).toBe("CompoundFileFormatError"); }); - it("throws for input shorter than the 512-byte header", () => { - expectFormatError( - new Uint8Array([ - 0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1, 0x00, 0x00, - ]), + it("throws its exact message for bytes without the compound-file signature", () => { + expect(() => readCompoundFile(enc("not a compound file at all"))).toThrow( + "readCompoundFile input does not carry the compound-file signature (leading magic bytes are not D0 CF 11 E0 A1 B1 1A E1)", ); }); - it("throws for a header whose sector shift contradicts its major version", () => { - const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); - bytes[0x1a] = 4; // major version 4 ... - bytes[0x1e] = 9; // ... still declaring 512-byte sectors, which version 4 forbids - expectFormatError(bytes); + it("throws naming the exact byte count for input shorter than the 512-byte header", () => { + const bytes = new Uint8Array([ + 0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1, 0x00, 0x00, + ]); + expect(() => readCompoundFile(bytes)).toThrow( + `compound file is ${bytes.length} bytes, shorter than the fixed 512-byte header`, + ); }); - it("throws for a big-endian byte-order field", () => { - const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); + it("accepts input exactly the 512-byte header's own length, rejecting it only for having no sector past it", () => { + // Exactly HEADER_SIZE bytes must not trip the "shorter than the header" check (bytes.length < HEADER_SIZE is false at equality) -- it fails a later, distinct check instead (no complete sector follows the header), proving the boundary itself is inclusive. + const bytes = compoundFile([]).slice(0, 512); + expect(() => readCompoundFile(bytes)).toThrow( + "compound file holds no complete 512-byte sector after its header", + ); + }); + + it("throws naming the exact declared major version when it is neither 3 nor 4", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + bytes[0x1a] = 5; + expect(() => readCompoundFile(bytes)).toThrow( + "compound file major version 5 is not 3 or 4", + ); + }); + + it("throws its exact message for a big-endian byte-order field", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); // The field holds FE FF (little-endian 0xFFFE); swapping to FF FE reads as 0xFEFF, the big-endian marker this reader refuses. bytes[0x1c] = 0xff; bytes[0x1d] = 0xfe; - expectFormatError(bytes); + expect(() => readCompoundFile(bytes)).toThrow( + "compound file byte order is not little-endian", + ); + }); + + it("throws naming both the found sector shift and the major version it contradicts, in both directions", () => { + const v3 = compoundFile([{ path: "A", bytes: enc("x") }], { + majorVersion: 3, + }); + v3[0x1e] = 12; // version 3 declaring 4096-byte sectors, which version 3 forbids + expect(() => readCompoundFile(v3)).toThrow( + "compound file sector shift 2^12 does not match major version 3 (version 3 requires 512-byte sectors, version 4 requires 4096-byte)", + ); + + const v4 = compoundFile([{ path: "A", bytes: enc("x") }], { + majorVersion: 4, + }); + v4[0x1e] = 9; // version 4 declaring 512-byte sectors, which version 4 forbids + expect(() => readCompoundFile(v4)).toThrow( + "compound file sector shift 2^9 does not match major version 4 (version 3 requires 512-byte sectors, version 4 requires 4096-byte)", + ); + }); + + it("throws naming the exact mini sector shift when it is not the mandated 64-byte mini sector", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + bytes[0x20] = 5; // 2^5 = 32, not the mandated 64 + expect(() => readCompoundFile(bytes)).toThrow( + "compound file mini sector shift 2^5 is not the mandated 64-byte mini sector", + ); + }); + + it("accepts a mini stream cutoff exactly at the mini sector size, and rejects one byte below it", () => { + const atCutoff = compoundFile([{ path: "A", bytes: enc("x") }]); + new DataView(atCutoff.buffer).setUint32(0x38, 64, true); // exactly the 64-byte mini sector + expect(() => readCompoundFile(atCutoff)).not.toThrow(); + + const belowCutoff = compoundFile([{ path: "A", bytes: enc("x") }]); + new DataView(belowCutoff.buffer).setUint32(0x38, 63, true); + expect(() => readCompoundFile(belowCutoff)).toThrow( + "compound file mini stream cutoff 63 is smaller than the 64-byte mini sector itself", + ); + }); + + it("throws naming the exact sector size when the file holds no complete sector after its header", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]).slice( + 0, + HEADER_BYTES + 100, // less than one whole 512-byte sector past the header + ); + expect(() => readCompoundFile(bytes)).toThrow( + "compound file holds no complete 512-byte sector after its header", + ); + }); + + it("accepts a file with exactly one complete sector past the header (sectorCount 1)", () => { + // Truncating compoundFile([])'s own 3-sector file (FAT, directory, nothing else) to just the header plus its one FAT sector leaves sectorCount === 1 -- the boundary sectorCount < 1 must not reject, so the failure instead comes from the directory chain stepping onto sector 1, which this truncation removed. + const bytes = compoundFile([]).slice(0, HEADER_BYTES + FAT_SECTOR_BYTES); + expect(() => readCompoundFile(bytes)).toThrow( + "a FAT chain steps to sector 1, which is outside the file's 1 sectors", + ); + }); + + it("throws its exact message for a DIFAT chain of exactly one sector that terminates without cycling", () => { + // A hand-built fixture, not compoundFile()/writeCompoundFile() output: both always keep the DIFAT inside the header's own 109-entry array, so a genuinely chained DIFAT walk of a KNOWN, minimal length has to be built directly. sectorCount is pinned to 1 (one sector past the header) so the chain-walk's own iteration counter reaches its "sectorCount visited" boundary on the very first, legitimate, non-repeating sector -- proving the counter's bound is inclusive of that many sectors, not exclusive. + const bytes = new Uint8Array(HEADER_BYTES + 512); + const view = new DataView(bytes.buffer); + bytes.set([0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1], 0); + view.setUint16(0x1a, 3, true); // majorVersion 3 + view.setUint16(0x1c, 0xfffe, true); // byte order + view.setUint16(0x1e, 9, true); // sectorShift: 512-byte sectors + view.setUint16(0x20, 6, true); // miniSectorShift: 64-byte mini sectors + view.setUint32(0x38, 64, true); // miniStreamCutoff, at least the mini sector size + view.setUint32(0x44, 0, true); // firstDifatSector: sector 0 is the sole chained DIFAT sector + // The header's own 109-entry DIFAT array: every slot FREESECT, so it contributes no FAT sectors of its own -- every candidate comes from the chained DIFAT sector below. + for (let i = 0; i < 109; i++) { + view.setUint32(0x4c + i * 4, 0xffffffff, true); + } + // Sector 0, the sole DIFAT sector: its 127 real entries all FREESECT, its own final slot (the next-DIFAT-sector pointer) set to ENDOFCHAIN so the chain is exactly one sector long and terminates cleanly. + + for (let i = 0; i < 127; i++) { + view.setUint32(HEADER_BYTES + i * 4, 0xffffffff, true); + } + view.setUint32(HEADER_BYTES + 127 * 4, 0xfffffffe, true); + // With no FAT sectors accepted from either source, the walk must reach the "no FAT sectors" check -- which it can only do if the one-sector DIFAT walk above was allowed to complete rather than being rejected as "too many sectors visited". + expect(() => readCompoundFile(bytes)).toThrow( + "compound file declares no FAT sectors, so no sector chain can be walked", + ); + }); + + it("names a chained DIFAT sector's own provenance when one of its entries is out of range", () => { + // Same minimal one-DIFAT-sector fixture as above, except entry 0 of the chained sector names a FAT sector one past the file's own single sector -- proving the thrown message cites "a DIFAT sector", not the header array's own provenance string (already covered by the header-DIFAT-array case elsewhere in this file). + const bytes = new Uint8Array(HEADER_BYTES + 512); + const view = new DataView(bytes.buffer); + bytes.set([0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1], 0); + view.setUint16(0x1a, 3, true); + view.setUint16(0x1c, 0xfffe, true); + view.setUint16(0x1e, 9, true); + view.setUint16(0x20, 6, true); + view.setUint32(0x38, 64, true); + view.setUint32(0x44, 0, true); + for (let i = 0; i < 109; i++) { + view.setUint32(0x4c + i * 4, 0xffffffff, true); + } + + view.setUint32(HEADER_BYTES, 1, true); // entry 0: sector 1, one past this file's single sector + for (let i = 1; i < 127; i++) { + view.setUint32(HEADER_BYTES + i * 4, 0xffffffff, true); + } + view.setUint32(HEADER_BYTES + 127 * 4, 0xfffffffe, true); + expect(() => readCompoundFile(bytes)).toThrow( + "a DIFAT sector names FAT sector 1, which is outside the file's 1 sectors", + ); + }); + + it("throws naming the exact sector when a FAT chain entry lies beyond the sectors the DIFAT named", () => { + // The DIFAT names only sector 0 as a FAT sector (a single 512-byte FAT table of 128 entries, valid sector numbers 0-127), but the file itself holds 200 sectors -- large enough that a directory chain starting at sector 128 passes the chain-walk's own file-bounds check yet steps past what the one declared FAT sector can address. + const totalSectors = 200; + const bytes = new Uint8Array(HEADER_BYTES + totalSectors * 512); + const view = new DataView(bytes.buffer); + bytes.set([0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1], 0); + view.setUint16(0x1a, 3, true); + view.setUint16(0x1c, 0xfffe, true); + view.setUint16(0x1e, 9, true); + view.setUint16(0x20, 6, true); + view.setUint32(0x38, 64, true); + view.setUint32(0x44, 0xfffffffe, true); // firstDifatSector: none, the header array alone suffices + view.setUint32(0x4c, 0, true); // header DIFAT[0]: sector 0 is the sole FAT sector + for (let i = 1; i < 109; i++) { + view.setUint32(0x4c + i * 4, 0xffffffff, true); + } + view.setUint32(0x30, 128, true); // firstDirectorySector: sector 128, past the FAT's own 128-entry coverage + expect(() => readCompoundFile(bytes)).toThrow( + "FAT entry for sector 128 lies beyond the sectors the DIFAT named", + ); + }); + + it("accepts a FAT entry read exactly at the declared FAT sectors' own last valid offset", () => { + // Sector 127 is the very last entry a single 512-byte FAT sector addresses (128 four-byte entries, indices 0-127) -- the boundary offset + 4 > fatBytes.length must not reject it. Its own FAT entry (left as zero from the allocation) reads back as 0, not ENDOFCHAIN, so the chain would loop forever if extended; instead its slot is set to ENDOFCHAIN directly so the chain resolves to one bare sector, whose all-zero directory contents fail a later, distinct check. + const totalSectors = 200; + const bytes = new Uint8Array(HEADER_BYTES + totalSectors * 512); + const view = new DataView(bytes.buffer); + bytes.set([0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1], 0); + view.setUint16(0x1a, 3, true); + view.setUint16(0x1c, 0xfffe, true); + view.setUint16(0x1e, 9, true); + view.setUint16(0x20, 6, true); + view.setUint32(0x38, 64, true); + view.setUint32(0x44, 0xfffffffe, true); + view.setUint32(0x4c, 0, true); + for (let i = 1; i < 109; i++) { + view.setUint32(0x4c + i * 4, 0xffffffff, true); + } + view.setUint32(0x30, 127, true); // firstDirectorySector: sector 127, the FAT's own last addressable entry + // Sector 0's own raw bytes double as the FAT table; entry 127 (the directory chain's own continuation) is set to ENDOFCHAIN so the chain is exactly one sector long. + view.setUint32(HEADER_BYTES + 127 * 4, 0xfffffffe, true); + expect(() => readCompoundFile(bytes)).toThrow( + "the first directory entry is not the root storage entry (object type 5), as [MS-CFB] 2.6.1 requires", + ); }); it("throws for a truncated file (sectors the header references are gone)", () => { @@ -134,11 +480,106 @@ describe("readCompoundFile malformed-input handling", () => { expectFormatError(bytes.slice(0, 700)); }); - it("throws for a DIFAT entry naming a sector outside the file", () => { + it("throws naming the header DIFAT array by name, and the exact sector/count, one sector past the file's own total", () => { const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); + // This fixture's own file-total sector count (computed the same way the reader itself derives it: whole sectorSize-byte sectors after the header). + const sectorCount = Math.floor(bytes.length / 512) - 1; const view = new DataView(bytes.buffer); - view.setUint32(0x4c, 0x0000ff00, true); // header DIFAT[0] -> far beyond the file's sector count - expectFormatError(bytes); + view.setUint32(0x4c, sectorCount, true); // header DIFAT[0] -> exactly one past the last valid sector + expect(() => readCompoundFile(bytes)).toThrow( + `the header DIFAT array names FAT sector ${sectorCount}, which is outside the file's ${sectorCount} sectors`, + ); + }); + + it("throws for a DIFAT chain entry naming a sector outside the file", () => { + // test-support/cfb.ts's own compoundFile never chains a DIFAT sector (its header comment says so: the DIFAT always fits the header's 109-entry array). Corrupting a DIFAT-chain entry specifically needs a file that genuinely has one, so this reaches for ../cfb/write.ts's writeCompoundFile instead -- not to test a round trip (write.test.ts already does that), but purely as a source of valid DIFAT-chained bytes to corrupt one byte of, exactly like every other case in this block corrupts a compoundFile()-built fixture. + const payload = new Uint8Array(8 * 1024 * 1024); + const bytes = writeCompoundFile([{ path: "WordDocument", bytes: payload }]); + const view = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength); + const sectorShift = view.getUint16(0x1e, true); + const sectorSize = 1 << sectorShift; + const firstDifatSector = view.getUint32(0x44, true); + expect(firstDifatSector).not.toBe(0xfffffffe); // sanity: this fixture really does chain a DIFAT sector + const sectorCount = Math.floor(bytes.length / sectorSize) - 1; + const entriesPerDifatSector = sectorSize / 4 - 1; + // Corrupt the chained DIFAT sector's own final slot -- its next-DIFAT-sector pointer, ENDOFCHAIN in this one-DIFAT-sector fixture -- to point past the file. This is the DIFAT chain-walk's own sector-number check (on the sector named IN the chain), not acceptFatSector's check on an ordinary FAT-index entry within it. + view.setUint32( + (firstDifatSector + 1) * sectorSize + entriesPerDifatSector * 4, + sectorCount, + true, + ); + expect(() => readCompoundFile(bytes)).toThrow( + `the DIFAT chain names sector ${sectorCount}, which is outside the file's ${sectorCount} sectors`, + ); + }); + + it("throws when the DIFAT chain visits more sectors than the file holds", () => { + // Same rationale as the case above: a genuine DIFAT-chained fixture is needed to corrupt, which only ../cfb/write.ts's writer currently produces. + const payload = new Uint8Array(8 * 1024 * 1024); + const bytes = writeCompoundFile([{ path: "WordDocument", bytes: payload }]); + const view = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength); + const sectorShift = view.getUint16(0x1e, true); + const sectorSize = 1 << sectorShift; + const firstDifatSector = view.getUint32(0x44, true); + const entriesPerDifatSector = sectorSize / 4 - 1; + // The chained DIFAT sector's own final slot (its own next-DIFAT-sector pointer), corrupted to point back at itself rather than ENDOFCHAIN or a genuinely later sector -- an infinite chain that must trip the visited-sector-count guard rather than looping forever. + view.setUint32( + (firstDifatSector + 1) * sectorSize + entriesPerDifatSector * 4, + firstDifatSector, + true, + ); + expect(() => readCompoundFile(bytes)).toThrow( + "the DIFAT chain visits more sectors than the file holds, so it must cycle", + ); + }); + + it("throws its exact message when every header DIFAT slot is FREESECT and no DIFAT chain names any FAT sector", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + for (let i = 0; i < 109; i++) { + view.setUint32(0x4c + i * 4, 0xffffffff, true); // FREESECT + } + expect(() => readCompoundFile(bytes)).toThrow( + "compound file declares no FAT sectors, so no sector chain can be walked", + ); + }); + + it("throws naming the exact sector and count for a FAT chain stepping outside the file", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); + const sectorCount = Math.floor(bytes.length / 512) - 1; + const view = new DataView(bytes.buffer); + // The stream's first data sector is sector 2 (FAT at 0, directory at 1); point its own FAT entry one sector past the file's own total. + view.setUint32(512 + 2 * 4, sectorCount, true); + expect(() => readCompoundFile(bytes)).toThrow( + `a FAT chain steps to sector ${sectorCount}, which is outside the file's ${sectorCount} sectors`, + ); + }); + + it("throws naming the exact sector and its role-marker entry for a FAT chain stepping onto a FATSECT/DIFSECT slot", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); + const view = new DataView(bytes.buffer); + view.setUint32(512 + 2 * 4, 0xfffffffd, true); // FATSECT, not a chain continuation + expect(() => readCompoundFile(bytes)).toThrow( + "a FAT chain steps to sector 2's entry 4294967293, which is a sector-role marker, not a chain continuation", + ); + }); + + it("throws for a FAT chain stepping onto a FREESECT slot", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); + const view = new DataView(bytes.buffer); + view.setUint32(512 + 2 * 4, 0xffffffff, true); // FREESECT, an unused slot, not a chain continuation + expect(() => readCompoundFile(bytes)).toThrow( + "a FAT chain steps to sector 2's entry 4294967295, which is a sector-role marker, not a chain continuation", + ); + }); + + it("throws for a FAT chain stepping onto a DIFSECT slot", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); + const view = new DataView(bytes.buffer); + view.setUint32(512 + 2 * 4, 0xfffffffc, true); // DIFSECT, not a chain continuation + expect(() => readCompoundFile(bytes)).toThrow( + "a FAT chain steps to sector 2's entry 4294967292, which is a sector-role marker, not a chain continuation", + ); }); it("throws for a FAT chain that cycles", () => { @@ -146,16 +587,206 @@ describe("readCompoundFile malformed-input handling", () => { // The stream's first data sector is sector 2 (FAT at 0, directory at 1); point its FAT entry back at itself so the chain never reaches ENDOFCHAIN. const view = new DataView(bytes.buffer); view.setUint32(512 + 2 * 4, 2, true); - expectFormatError(bytes); + expect(() => readCompoundFile(bytes)).toThrow( + "a FAT chain visits more sectors than the file holds, so it must cycle", + ); + }); + + it("throws its exact message for an empty directory chain", () => { + // A directory whose own single sector's chain entry is corrupted straight to ENDOFCHAIN, making chainBytes(firstDirectorySector) return zero bytes -- the directory's FAT chain, not its content, is what determines emptiness here. + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + view.setUint32(0x30, 0xfffffffe, true); // firstDirectorySector := ENDOFCHAIN + expect(() => readCompoundFile(bytes)).toThrow( + "compound file has an empty directory chain", + ); + }); + + it("throws its exact message when the first directory entry is not the root storage type", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + bytes[entryOffset(0) + 0x42] = 1; // root entry's own object type, corrupted from 5 (root) to 1 (storage) + expect(() => readCompoundFile(bytes)).toThrow( + "the first directory entry is not the root storage entry (object type 5), as [MS-CFB] 2.6.1 requires", + ); }); - it("throws for a stream whose declared size exceeds its chain", () => { + it("throws naming the exact mini sector and count for a mini-FAT chain stepping outside the mini stream", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); // 1 byte -> 1 mini sector + const view = new DataView(bytes.buffer); + const miniFatStart = view.getUint32(0x3c, true); + // Pointed exactly one mini sector past the mini stream's single sector -- the boundary itself, not merely somewhere comfortably out of range, so a >= mutated to > cannot let it through unnoticed. + view.setUint32((miniFatStart + 1) * 512, 1, true); + expect(() => readCompoundFile(bytes)).toThrow( + "a mini-FAT chain steps to mini sector 1, which is outside the mini stream's 1 mini sectors", + ); + }); + + it("throws its exact message for a mini-FAT chain that cycles", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + const miniFatStart = view.getUint32(0x3c, true); + view.setUint32((miniFatStart + 1) * 512, 0, true); // the one mini sector's own entry, pointed back at itself + expect(() => readCompoundFile(bytes)).toThrow( + "a mini-FAT chain visits more mini sectors than the mini stream holds, so it must cycle", + ); + }); + + it("throws naming the exact mini sector and its role-marker entry for a mini-FAT chain stepping onto a FATSECT/DIFSECT slot", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + const miniFatStart = view.getUint32(0x3c, true); + view.setUint32((miniFatStart + 1) * 512, 0xfffffffc, true); // DIFSECT + expect(() => readCompoundFile(bytes)).toThrow( + "a mini-FAT chain steps to mini sector 0's entry 4294967292, which is a sector-role marker, not a chain continuation", + ); + }); + + it("throws for a mini-FAT chain stepping onto a FREESECT slot", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + const miniFatStart = view.getUint32(0x3c, true); + view.setUint32((miniFatStart + 1) * 512, 0xffffffff, true); // FREESECT + expect(() => readCompoundFile(bytes)).toThrow( + "a mini-FAT chain steps to mini sector 0's entry 4294967295, which is a sector-role marker, not a chain continuation", + ); + }); + + it("throws for a mini-FAT chain stepping onto a FATSECT slot", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + const miniFatStart = view.getUint32(0x3c, true); + view.setUint32((miniFatStart + 1) * 512, 0xfffffffd, true); // FATSECT + expect(() => readCompoundFile(bytes)).toThrow( + "a mini-FAT chain steps to mini sector 0's entry 4294967293, which is a sector-role marker, not a chain continuation", + ); + }); + + it("throws naming the entry, its declared size, and its chain's real length when the declared size exceeds it", () => { const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); // Entry 1 is the stream; inflate its declared size to a figure no chain in this small file can fill. const view = new DataView(bytes.buffer); - const entryOffset = 512 + 512 + 1 * 128; // header + FAT sector + directory sector, entry 1 - view.setUint32(entryOffset + 0x78, 0x00ffffff, true); - expectFormatError(bytes); + view.setUint32(entryOffset(1) + 0x78, 0x00ffffff, true); + expect(() => readCompoundFile(bytes)).toThrow( + "stream 'A' declares 16777215 bytes but its chain holds only", + ); + }); + + it("combines the size field's low and high 32-bit halves by addition and a 2^32 multiplier", () => { + // A high half of 1 with a zero low half declares exactly 4294967296 bytes (2^32) -- a size only the high half's own *4294967296 term, added to the low half, can produce. Corrupting either the operator (+ to -) or the multiplier (* to /) yields a size so different (negative, or a tiny fraction) that the mini-resident extraction path below it never throws at all, rather than citing this exact figure. + const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); + const view = new DataView(bytes.buffer); + view.setUint32(entryOffset(1) + 0x78, 0, true); + view.setUint32(entryOffset(1) + 0x7c, 1, true); + expect(() => readCompoundFile(bytes)).toThrow( + "stream 'A' declares 4294967296 bytes but its chain holds only", + ); + }); + + it("accepts a declared size of exactly Number.MAX_SAFE_INTEGER, rejecting only one byte past it", () => { + // Number.MAX_SAFE_INTEGER itself must not trip the "beyond the integer range" guard -- the boundary is inclusive -- so this fixture's own tiny chain instead fails the (distinct) declared-size-versus-chain-length check. + const atLimit = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); + const atLimitView = new DataView(atLimit.buffer); + atLimitView.setUint32(entryOffset(1) + 0x78, 0xffffffff, true); + atLimitView.setUint32(entryOffset(1) + 0x7c, 2097151, true); // together: exactly Number.MAX_SAFE_INTEGER + expect(() => readCompoundFile(atLimit)).toThrow( + "stream 'A' declares 9007199254740991 bytes but its chain holds only", + ); + + const pastLimit = compoundFile([ + { path: "A", bytes: enc("x".repeat(5000)) }, + ]); + const pastLimitView = new DataView(pastLimit.buffer); + pastLimitView.setUint32(entryOffset(1) + 0x78, 0, true); + pastLimitView.setUint32(entryOffset(1) + 0x7c, 2097152, true); // together: Number.MAX_SAFE_INTEGER + 1 + expect(() => readCompoundFile(pastLimit)).toThrow( + "stream 'A' declares a size beyond the integer range this reader addresses", + ); + }); + + it("extracts a zero-length stream without ever inspecting its own starting sector", () => { + // [MS-CFB] 2.6.1: a zero-length stream's starting sector is meaningless, so extractStream must return empty without walking anything -- corrupt entry 1's own startSector to a mini sector far outside this 1-mini-sector fixture's single valid one, so a version that DID walk it would throw, while the real early return never gets that far. + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + view.setUint32(entryOffset(1) + 0x78, 0, true); // size := 0 + view.setUint32(entryOffset(1) + 0x74, 99, true); // startSector := an out-of-range mini sector + const streams = readCompoundFile(bytes); + expect(streams).toEqual([{ path: "A", bytes: new Uint8Array(0) }]); + }); + + it("throws naming the exact entry id and directory size when the directory tree links outside the directory's own entries", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + // The root's own child link (its sole real entry, id 1) corrupted to name an entry id past this file's directory. entryCount reflects the whole padded 512-byte directory sector (4 entries of 128 bytes each), not just the 2 real ones (root + A), so it is 4, not 2. + const view = new DataView(bytes.buffer); + view.setUint32(entryOffset(0) + 0x4c, 9, true); + expect(() => readCompoundFile(bytes)).toThrow( + "the directory tree links to entry 9, which is outside the directory's 4 entries", + ); + }); + + it("throws its exact message when the directory tree reaches the same entry twice", () => { + const bytes = compoundFile([ + { path: "A", bytes: enc("x") }, + { path: "B", bytes: enc("y") }, + ]); + // A's own right sibling (entry 1's rightId) already names B (entry 2); make B's own right sibling point back at A too, so the tree visits entry 1 a second time. + const view = new DataView(bytes.buffer); + view.setUint32(entryOffset(2) + 0x48, 1, true); + expect(() => readCompoundFile(bytes)).toThrow( + "the directory tree reaches entry 1 twice, so its sibling and child links cycle", + ); + }); + + it("throws naming the exact entry id and declared name length when it is out of the valid 2-64 even-byte range", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + view.setUint16(entryOffset(1) + 0x40, 65, true); // odd, and past 64 + expect(() => readCompoundFile(bytes)).toThrow( + "directory entry 1 declares name length 65, which is not an even byte count between 2 and 64", + ); + }); + + it.each([ + [0, "below the 2-byte minimum, and even"], + [1, "below the 2-byte minimum, and odd"], + [3, "within range, but odd"], + [63, "within range, but odd"], + [66, "even, but past the 64-byte maximum"], + ])("throws for a declared name length of %i (%s)", (nameLength: number) => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + view.setUint16(entryOffset(1) + 0x40, nameLength, true); + expect(() => readCompoundFile(bytes)).toThrow( + `directory entry 1 declares name length ${nameLength}, which is not an even byte count between 2 and 64`, + ); + }); + + it("accepts a declared name length of exactly 2 (an empty name) and exactly 64 (a 31-character name)", () => { + // 2 is the smallest legal name-length field (the terminating null alone, no real characters); 64 is the largest (31 UTF-16 code units plus the null). Both boundaries must be accepted, not rejected alongside the values one step outside them above. + const empty = compoundFile([{ path: "A", bytes: enc("x") }]); + new DataView(empty.buffer).setUint16(entryOffset(1) + 0x40, 2, true); + expect(readCompoundFile(empty).map((s) => s.path)).toEqual([""]); + + const maxName = compoundFile([{ path: "A".repeat(31), bytes: enc("x") }]); + expect(readCompoundFile(maxName).map((s) => s.path)).toEqual([ + "A".repeat(31), + ]); + }); + + it("throws naming the exact entry id, name, and object type for an unsupported directory entry type", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + bytes[entryOffset(1) + 0x42] = 9; // neither storage (1), stream (2), nor root (5) + expect(() => readCompoundFile(bytes)).toThrow( + "directory entry 1 ('A') carries object type 9, which is not a storage (1), stream (2), or root (5) entry", + ); + }); + + it("throws its exact message when the tree reaches a second root-typed (object type 5) entry", () => { + // Object type 5 passes the descend-stage's own storage/stream/root check (line ~384) unchanged, since ROOT is one of the three types it accepts -- it is only the self-stage's own switch, which explicitly handles STREAM and STORAGE alone, that has no case for a second type-5 entry reached anywhere but the directory's own id-0 slot. A's own name and length stay genuinely valid, so this exercises that check in isolation from the name-length and object-type-acceptance checks above it. + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + bytes[entryOffset(1) + 0x42] = 5; // A's own object type, corrupted from STREAM (2) to ROOT (5) + expect(() => readCompoundFile(bytes)).toThrow( + "the directory tree reaches entry A, which is not a storage or stream entry", + ); }); it("throws for a directory tree whose sibling links cycle", () => { @@ -165,21 +796,29 @@ describe("readCompoundFile malformed-input handling", () => { ]); // Entries 1 (A) and 2 (B) are siblings chained 1 -> 2; point B's right sibling back at A. const view = new DataView(bytes.buffer); - const entryOffset = (id: number) => 512 + 512 + id * 128; view.setUint32(entryOffset(2) + 0x48, 1, true); expectFormatError(bytes); }); - it("throws when the cumulative extracted size exceeds the configured budget", () => { + it("throws naming the exact budget and entry name when the cumulative extracted size exceeds it", () => { // Two 5000-byte streams with a 6000-byte budget: the second extraction tips the cumulative total over, so the whole read fails rather than returning a partial listing -- the same stance archive-codec's ZIP walk takes on its guards, and for the same reason (a hostile FAT can alias one sector into many streams, multiplying extraction beyond the file's own size). const bytes = compoundFile([ { path: "A", bytes: enc("x".repeat(5000)) }, { path: "B", bytes: enc("y".repeat(5000)) }, ]); expect(() => readCompoundFile(bytes, { maxTotalBytes: 6000 })).toThrow( - CompoundFileFormatError, + "cumulative extracted stream size exceeded the 6000-byte budget at 'B'", ); // The same file under the default budget reads fine -- the guard fires on the budget, not on the structure. expect(readCompoundFile(bytes)).toHaveLength(2); }); + + it("accepts a cumulative extracted size exactly equal to the budget", () => { + // 5000 bytes twice is exactly 10000 -- the boundary itself must be allowed, not just amounts strictly under it. + const bytes = compoundFile([ + { path: "A", bytes: enc("x".repeat(5000)) }, + { path: "B", bytes: enc("y".repeat(5000)) }, + ]); + expect(readCompoundFile(bytes, { maxTotalBytes: 10000 })).toHaveLength(2); + }); }); diff --git a/packages/archive-codec/src/cfb/read.ts b/packages/archive-codec/src/cfb/read.ts index f246d4c0c7..cfa06c9093 100644 --- a/packages/archive-codec/src/cfb/read.ts +++ b/packages/archive-codec/src/cfb/read.ts @@ -179,9 +179,10 @@ export function readCompoundFile( } const fat = new DataView(fatBytes.buffer); + // No offset<0 guard: sector is always a chain's own start (a u32 header/entry read) or a prior fatEntry return (itself a u32 read), so it can never actually be negative -- a defensive check against an input this closure never receives. const fatEntry = (sector: number): number => { const offset = sector * 4; - if (offset < 0 || offset + 4 > fatBytes.length) { + if (offset + 4 > fatBytes.length) { throw new CompoundFileFormatError( `FAT entry for sector ${sector} lies beyond the sectors the DIFAT named`, ); @@ -189,8 +190,10 @@ export function readCompoundFile( return fat.getUint32(offset, true); }; + // Cycle detection is a direct visited-set membership check, not a count-based pigeonhole bound (ids.length >= sectorCount): the two are equivalent in the cases they actually reject, but a revisited sector's own FAT entry is deterministic, so a count-based bound only ever fires one or more full cycles after the true repeat -- every intervening iteration replays a step this same chain already took, producing byte-for-byte the same eventual outcome regardless of exactly which iteration trips the bound. A membership check instead fires on the exact iteration a sector is seen twice, matching the directory tree walk's own visited-set below. const chainSectorIds = (start: number): number[] => { const ids: number[] = []; + const visited = new Set(); let current = start; while (current !== ENDOFCHAIN) { if (current >= sectorCount) { @@ -198,11 +201,12 @@ export function readCompoundFile( `a FAT chain steps to sector ${current}, which is outside the file's ${sectorCount} sectors`, ); } - if (ids.length >= sectorCount) { + if (visited.has(current)) { throw new CompoundFileFormatError( "a FAT chain visits more sectors than the file holds, so it must cycle", ); } + visited.add(current); ids.push(current); const next = fatEntry(current); if (next === FREESECT || next === FATSECT || next === DIFSECT) { @@ -268,8 +272,10 @@ export function readCompoundFile( const miniFatBytes = chainBytes(firstMiniFatSector); const miniFat = new DataView(miniFatBytes.buffer); + // Same visited-set cycle detection as chainSectorIds above, and for the same reason. const miniChainSectorIds = (start: number): number[] => { const ids: number[] = []; + const visited = new Set(); let current = start; while (current !== ENDOFCHAIN) { if (current >= miniSectorCount) { @@ -277,11 +283,12 @@ export function readCompoundFile( `a mini-FAT chain steps to mini sector ${current}, which is outside the mini stream's ${miniSectorCount} mini sectors`, ); } - if (ids.length >= miniSectorCount) { + if (visited.has(current)) { throw new CompoundFileFormatError( "a mini-FAT chain visits more mini sectors than the mini stream holds, so it must cycle", ); } + visited.add(current); ids.push(current); const next = miniFat.getUint32(current * 4, true); if (next === FREESECT || next === FATSECT || next === DIFSECT) { @@ -359,8 +366,8 @@ export function readCompoundFile( continue; } const entry = entries[id]; - // First encounter: bounds, cycle, and shape validation happen here only -- the self-stage revisit of the same entry must not trip the visited set. - if (id >= entryCount || entry === undefined) { + // First encounter: bounds, cycle, and shape validation happen here only -- the self-stage revisit of the same entry must not trip the visited set. No separate id >= entryCount guard: entries is a dense array of exactly entryCount elements, so any id at or past that length reads back as undefined regardless of how large id is -- the entry === undefined check alone already catches every out-of-range id. + if (entry === undefined) { throw new CompoundFileFormatError( `the directory tree links to entry ${id}, which is outside the directory's ${entryCount} entries`, ); diff --git a/packages/archive-codec/src/cfb/write.test.ts b/packages/archive-codec/src/cfb/write.test.ts index 01d923a3e4..59eb65b57a 100644 --- a/packages/archive-codec/src/cfb/write.test.ts +++ b/packages/archive-codec/src/cfb/write.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from "vitest"; import { type CompoundFileStream, readCompoundFile } from "./read"; -import { CompoundFileWriteError, writeCompoundFile } from "./write"; +import { + CompoundFileWriteError, + deepestDepth, + exceedsVersion3StreamCeiling, + highSizeWord, + writeCompoundFile, +} from "./write"; // Coverage for the [MS-CFB] writer (src/cfb/write.ts). Two kinds of check, deliberately kept separate: // @@ -195,10 +201,12 @@ function expectRedBlackTree( } describe("writeCompoundFile header and sector layout", () => { - // One 5-byte stream: small enough for the mini stream, so the file is the minimal shape that still exercises every structure -- header, one FAT sector, one directory sector, the mini stream, and the mini FAT. Every expectation below is derived from the spec's field tables, then checked against the layout this writer commits to: sector 0 FAT, sector 1 directory, sector 2 mini stream, sector 3 mini FAT. - const minimal = writeCompoundFile([stream("Foo", enc("hello"))]); + // One 5-byte stream: small enough for the mini stream, so the file is the minimal shape that still exercises every structure -- header, one FAT sector, one directory sector, the mini stream, and the mini FAT. Every expectation below is derived from the spec's field tables, then checked against the layout this writer commits to: sector 0 FAT, sector 1 directory, sector 2 mini stream, sector 3 mini FAT. Built fresh inside each it() rather than shared at describe-top-level: a shared const built once at module/describe setup time runs before any specific test, so Stryker's per-test coverage tracker cannot attribute a mutation in writeCompoundFile's own body to whichever assertion below would actually catch it, and every mutant it introduces there is misreported as surviving regardless of whether a real test kills it. + const minimalFixture = (): Uint8Array => + writeCompoundFile([stream("Foo", enc("hello"))]); it("writes the header signature, CLSID, versions, and byte order [MS-CFB] 2.2", () => { + const minimal = minimalFixture(); expect([...minimal.subarray(0, 8)]).toEqual([ 0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1, ]); @@ -216,6 +224,7 @@ describe("writeCompoundFile header and sector layout", () => { }); it("writes the sector-count and location fields the minimal file's layout implies", () => { + const minimal = minimalFixture(); expect(u32(minimal, 0x28)).toBe(0); // number of directory sectors MUST be zero for major version 3 expect(u32(minimal, 0x2c)).toBe(1); // one FAT sector maps all four sectors of this file expect(u32(minimal, 0x30)).toBe(1); // first directory sector @@ -228,6 +237,7 @@ describe("writeCompoundFile header and sector layout", () => { }); it("writes the header DIFAT array: the FAT sector locations in order, then FREESECT padding", () => { + const minimal = minimalFixture(); expect(u32(minimal, 0x4c)).toBe(0); for (let i = 1; i < 109; i++) { expect(u32(minimal, 0x4c + i * 4)).toBe(FREESECT); @@ -236,10 +246,11 @@ describe("writeCompoundFile header and sector layout", () => { it("sizes the file at one header sector plus its sectors, and lays the sectors out contiguously", () => { // Sector N occupies bytes [(N + 1) * sectorSize, (N + 2) * sectorSize) ([MS-CFB] 2.3), so a four-sector version 3 file is 5 * 512 bytes. - expect(minimal.length).toBe(512 * 5); + expect(minimalFixture().length).toBe(512 * 5); }); it("marks the FAT sector as FATSECT and terminates every one-sector chain [MS-CFB] 2.3", () => { + const minimal = minimalFixture(); const fat = 512; expect(u32(minimal, fat + 0 * 4)).toBe(FATSECT); // sector 0 holds the FAT itself expect(u32(minimal, fat + 1 * 4)).toBe(ENDOFCHAIN); // directory @@ -252,6 +263,7 @@ describe("writeCompoundFile header and sector layout", () => { }); it("writes the root directory entry per [MS-CFB] 2.6.1/2.6.2", () => { + const minimal = minimalFixture(); const root = 512 * 2; expect( new TextDecoder("utf-16le").decode(minimal.subarray(root, root + 20)), @@ -275,6 +287,7 @@ describe("writeCompoundFile header and sector layout", () => { }); it("writes the stream directory entry, mini-resident because it is under the cutoff", () => { + const minimal = minimalFixture(); const entry = 512 * 2 + 128; expect( new TextDecoder("utf-16le").decode(minimal.subarray(entry, entry + 6)), @@ -290,6 +303,7 @@ describe("writeCompoundFile header and sector layout", () => { }); it("writes the unallocated directory entries padding the sector as object type 0 with NOSTREAM links", () => { + const minimal = minimalFixture(); for (const slot of [2, 3]) { const base = 512 * 2 + slot * 128; expect(u16(minimal, base + 0x40)).toBe(0); @@ -297,10 +311,14 @@ describe("writeCompoundFile header and sector layout", () => { expect(u32(minimal, base + 0x44)).toBe(NOSTREAM); expect(u32(minimal, base + 0x48)).toBe(NOSTREAM); expect(u32(minimal, base + 0x4c)).toBe(NOSTREAM); + // Nothing ever explicitly writes an unallocated entry's own size fields, so 0x78/0x7c must still read the zero the allocation started with. Slot 3's own 0x7c sits at this fixture's absolute offset 1532 -- one past where a with-DIFAT file's own chained-DIFAT-sector loop, run one sector too far, would land its stray terminator write, so this is also where such an overrun would first become visible. + expect(u32(minimal, base + 0x78)).toBe(0); + expect(u32(minimal, base + 0x7c)).toBe(0); } }); it("stores the small stream in the mini stream, zero-padded to a whole mini sector, chained by the mini FAT", () => { + const minimal = minimalFixture(); const miniStream = 512 * 3; expect([...minimal.subarray(miniStream, miniStream + 5)]).toEqual([ ...enc("hello"), @@ -595,8 +613,9 @@ describe("writeCompoundFile input validation", () => { }); it("rejects a name longer than the 32 code points the directory entry holds", () => { - expect(() => writeCompoundFile([stream("N".repeat(32), enc("x"))])).toThrow( - CompoundFileWriteError, + const name = "N".repeat(32); + expect(() => writeCompoundFile([stream(name, enc("x"))])).toThrow( + `'${name}' is 32 UTF-16 code points, more than the 31 a directory entry's name field holds alongside its terminating null (in stream path ${JSON.stringify(name)})`, ); expect(() => writeCompoundFile([stream("N".repeat(31), enc("x"))]), @@ -606,7 +625,7 @@ describe("writeCompoundFile input validation", () => { it("rejects an empty path or an empty path segment", () => { for (const path of ["", "/Leading", "Trailing/", "Double//Segment"]) { expect(() => writeCompoundFile([stream(path, enc("x"))])).toThrow( - CompoundFileWriteError, + `stream path ${JSON.stringify(path)} has an empty name segment; every segment must name a storage, and the last must name the stream`, ); } }); @@ -614,7 +633,9 @@ describe("writeCompoundFile input validation", () => { it("rejects the same path supplied twice", () => { expect(() => writeCompoundFile([stream("Dup", enc("1")), stream("Dup", enc("2"))]), - ).toThrow(CompoundFileWriteError); + ).toThrow( + `stream path "Dup" collides with 'Dup', which the file already holds in the same storage ([MS-CFB] 2.6.4 requires siblings to have unique names)`, + ); }); it("rejects a path that needs one name to be both a storage and a stream", () => { @@ -623,7 +644,9 @@ describe("writeCompoundFile input validation", () => { stream("Thing", enc("1")), stream("Thing/Inner", enc("2")), ]), - ).toThrow(CompoundFileWriteError); + ).toThrow( + `stream path "Thing/Inner" needs 'Thing' to be a storage, but the file already holds a stream by that name`, + ); expect(() => writeCompoundFile([ stream("Thing/Inner", enc("2")), @@ -637,4 +660,258 @@ describe("writeCompoundFile input validation", () => { /bad:name/, ); }); + + it("names every thrown error CompoundFileWriteError, not merely an instance of it", () => { + let caught: unknown; + try { + writeCompoundFile([stream("bad:name", enc("x"))]); + } catch (error) { + caught = error; + } + expect((caught as Error).name).toBe("CompoundFileWriteError"); + }); + + it("rejects a version 3 stream one byte past the 0x80000000 ceiling, naming its exact size", () => { + // A real allocation, not a mock: V8 zero-fills a fresh Uint8Array lazily, so this is a fast, cheap way to prove the boundary and its thrown message against the writer's actual real input type, not a stand-in for it. + const oversized = new Uint8Array(0x80000000 + 1); + expect(() => writeCompoundFile([stream("Big", oversized)])).toThrow( + 'stream "Big" is 2147483649 bytes, past the 2147483648-byte ceiling [MS-CFB] 2.6.1 puts on a version 3 stream; write the file as version 4 instead', + ); + }); +}); + +describe("exceedsVersion3StreamCeiling", () => { + it("is false for a version 3 stream at or under the ceiling", () => { + expect(exceedsVersion3StreamCeiling(3, 0x80000000)).toBe(false); + expect(exceedsVersion3StreamCeiling(3, 0)).toBe(false); + }); + + it("is true for a version 3 stream one byte past the ceiling", () => { + expect(exceedsVersion3StreamCeiling(3, 0x80000001)).toBe(true); + }); + + it("is always false for version 4, whatever the byte length", () => { + expect(exceedsVersion3StreamCeiling(4, 0x80000001)).toBe(false); + expect(exceedsVersion3StreamCeiling(4, Number.MAX_SAFE_INTEGER)).toBe( + false, + ); + }); +}); + +describe("highSizeWord", () => { + it("is 0 for any size under 2^32", () => { + expect(highSizeWord(0)).toBe(0); + expect(highSizeWord(4294967295)).toBe(0); + }); + + it("divides by 2^32 and floors, not multiplies, for a size at and past the boundary", () => { + expect(highSizeWord(4294967296)).toBe(1); // exactly 2^32 + expect(highSizeWord(4294967296 * 2 + 500)).toBe(2); // past it, with a nonzero low remainder + }); +}); + +describe("deepestDepth", () => { + it("is 0 for a count of 0 or 1", () => { + expect(deepestDepth(0)).toBe(0); + expect(deepestDepth(1)).toBe(0); + }); + + it("is floor(log2(count)) for every count up to a few levels deep", () => { + expect(deepestDepth(2)).toBe(1); + expect(deepestDepth(3)).toBe(1); + expect(deepestDepth(4)).toBe(2); + expect(deepestDepth(7)).toBe(2); + expect(deepestDepth(8)).toBe(3); + expect(deepestDepth(63)).toBe(5); + expect(deepestDepth(64)).toBe(6); + }); +}); + +describe("writeCompoundFile sibling-name case mapping", () => { + it("sorts by the simple (single-code-point) uppercase mapping, not the lowercase one", () => { + // The Kelvin sign (U+212A) uppercases to itself (0x212A) but lowercases to plain 'k' (0x6B) -- verified directly against V8's own Intl-backed toUpperCase/toLowerCase. Comparing it against 'L' (0x4C upper, 0x6C lower) gives opposite orderings under the two mappings: uppercase puts the Kelvin sign after 'L' (0x212A > 0x4C), lowercase would put it before (0x6B < 0x6C). + const streams = readCompoundFile( + writeCompoundFile([stream("L", enc("1")), stream("K", enc("2"))]), + ); + expect(streams.map((s) => s.path)).toEqual(["L", "K"]); + }); + + it("leaves a code unit whose simple uppercase mapping expands to more than one character unchanged, rather than taking the expansion's first character", () => { + // 'ß' (U+00DF) uppercases to the two-character string "SS" under JS's FULL case mapping; [MS-CFB] 2.6.4's own SIMPLE (single-code-point) mapping leaves such a code unit as itself (0xDF) instead. Compared against 'T' (0x54 upper): the real, unexpanded 0xDF sorts after 'T', but the expansion's first character 'S' (0x53) would sort before it. + const streams = readCompoundFile( + writeCompoundFile([stream("T", enc("1")), stream("ß", enc("2"))]), + ); + expect(streams.map((s) => s.path)).toEqual(["T", "ß"]); + }); +}); + +describe("writeCompoundFile mini-stream sector allocation", () => { + it("writes ENDOFCHAIN as a zero-length entry's own starting sector, not a mini-stream offset", () => { + const entries = parseDirectory( + writeCompoundFile([stream("Empty", new Uint8Array(0))]), + ); + const entry = entries.find((e) => e.name === "Empty"); + expect(entry?.startSector).toBe(ENDOFCHAIN); + }); + + it("allocates each mini-resident stream's own starting mini sector sequentially, by its own byte length divided by the 64-byte mini sector, not multiplied by it", () => { + // Three same-length names (so [MS-CFB] 2.6.4's length-first ordering leaves them in plain alphabetical, i.e. insertion, order) whose mini-sector counts (ceil(length / 64)) are each individually distinguishable: 1, 2, and 1 mini sectors. A multiplication instead of division would inflate the running total by orders of magnitude after the very first stream, corrupting every later stream's own starting mini sector. + const entries = parseDirectory( + writeCompoundFile([ + stream("Aaa", new Uint8Array(30)), // ceil(30/64) = 1 mini sector + stream("Bbb", new Uint8Array(100)), // ceil(100/64) = 2 mini sectors + stream("Ccc", new Uint8Array(10)), // ceil(10/64) = 1 mini sector + ]), + ); + const startSectorOf = (name: string): number | undefined => + entries.find((e) => e.name === name)?.startSector; + expect(startSectorOf("Aaa")).toBe(0); + expect(startSectorOf("Bbb")).toBe(1); + expect(startSectorOf("Ccc")).toBe(3); + }); + + it("needs a second mini FAT sector once the mini stream passes 128 mini sectors, dividing not multiplying to compute it", () => { + // entriesPerFatSector is sectorSize / 4 = 128 for a version 3 (512-byte-sector) file, so a mini stream of exactly 129 mini sectors needs ceil(129 / 128) = 2 mini FAT sectors, not 1 -- and a multiplication in that division would instead compute an enormous, clearly-wrong sector count. + const miniSectorsNeeded = 129; + const streams = Array.from( + { length: miniSectorsNeeded }, + (_unused, i) => stream(`M${i}`, new Uint8Array(64)), // exactly one mini sector each + ); + const bytes = writeCompoundFile(streams); + const miniFatSectorCount = u32(bytes, 0x40); + expect(miniFatSectorCount).toBe(2); + const roundTripped = readCompoundFile(bytes); + expect(roundTripped).toHaveLength(miniSectorsNeeded); + }); +}); + +describe("writeCompoundFile FAT, mini-FAT, and DIFAT region padding", () => { + // 24 MiB forces three chained DIFAT sectors past the header's own 109-entry array ([MS-CFB] 2.5), not just one or two: the DIFAT-chaining loop's own next-sector arithmetic (difatStart + sector + 1) needs a NON-LAST sector at an index past 0 to distinguish from a subtly wrong variant, since at sector 0 every candidate formula agrees (any term multiplied, divided, or negated by 0 is 0), and a fixture with only two DIFAT sectors has no non-last sector other than 0. + // + // Built fresh inside each test, not shared via a describe-level beforeAll: Stryker's per-test coverage analysis only attributes code executed inside an it() body to that test -- a beforeAll hook runs outside every individual test's own tracked window, so a mutant reachable only through it (as this fixture's own DIFAT-chaining arithmetic is, nowhere else in this suite) gets no usable per-test coverage at all, confirmed directly against a live mutation run. Recomputing the same 24 MiB write per test costs a fraction of a second and buys correct attribution. + const sectorSize = 512; + const sectorOffset = (sector: number): number => (sector + 1) * sectorSize; + function bigDifatFixture(): { + bytes: Uint8Array; + fatSectorCount: number; + difatSectorCount: number; + difatStart: number; + miniFatSectorCount: number; + } { + const payload = new Uint8Array(24 * 1024 * 1024); + const bytes = writeCompoundFile([stream("WordDocument", payload)]); + return { + bytes, + fatSectorCount: u32(bytes, 0x2c), + difatSectorCount: u32(bytes, 0x48), + difatStart: u32(bytes, 0x44), + miniFatSectorCount: u32(bytes, 0x40), + }; + } + + it("needs more than one FAT sector and at least three chained DIFAT sectors for this fixture", () => { + // Sanity check on the fixture itself before trusting the boundary assertions below against it: at least three DIFAT sectors are what makes the DIFAT-chaining loop's own per-sector index and next-pointer arithmetic observable at all (see the fixture's own comment above). + const { fatSectorCount, difatSectorCount } = bigDifatFixture(); + expect(fatSectorCount).toBeGreaterThan(1); + expect(difatSectorCount).toBeGreaterThanOrEqual(3); + }); + + it("marks every FAT sector as FATSECT and every DIFAT sector as DIFSECT in the FAT table itself", () => { + // The FAT's own entry for each of its own sectors and each DIFAT sector is a role marker, never a chain continuation -- read directly from the FAT table (not merely inferred from the file round-tripping), since no reader ever follows a chain onto one of these sectors to notice a wrong marker there. + const { bytes, fatSectorCount, difatSectorCount, difatStart } = + bigDifatFixture(); + const entriesPerFatSector = sectorSize / 4; + const fatEntry = (sector: number): number => { + const holder = Math.floor(sector / entriesPerFatSector); + return u32( + bytes, + sectorOffset(holder) + (sector % entriesPerFatSector) * 4, + ); + }; + for (let sector = 0; sector < fatSectorCount; sector++) { + expect(fatEntry(sector)).toBe(FATSECT); + } + for ( + let sector = difatStart; + sector < difatStart + difatSectorCount; + sector++ + ) { + expect(fatEntry(sector)).toBe(DIFSECT); + } + }); + + it("fills the FAT's own unused tail entries with FREESECT, past the file's real total sector count", () => { + // The FAT addresses fatSectorCount * 128 sectors total (128 entries per 512-byte FAT sector); the file itself occupies exactly (bytes.length / sectorSize) - 1 real sectors (the header takes the first sectorSize bytes, uncounted). Every FAT entry beyond that real count is unused padding, and must read FREESECT. This writer lays FAT sectors out as physical sectors 0..fatSectorCount-1 (an identity mapping the header DIFAT array and any chained DIFAT sectors both merely restate), so the FAT sector holding a given sector's own entry is that sector's ordinal FAT-sector index directly, with no indirection needed. + const { bytes, fatSectorCount } = bigDifatFixture(); + const entriesPerFatSector = sectorSize / 4; + const totalRealSectors = bytes.length / sectorSize - 1; + const totalAddressableSectors = fatSectorCount * entriesPerFatSector; + expect(totalAddressableSectors).toBeGreaterThan(totalRealSectors); // otherwise this fixture has no padding tail left to check at all + for ( + let sector = totalRealSectors; + sector < totalAddressableSectors; + sector++ + ) { + const holder = Math.floor(sector / entriesPerFatSector); + expect( + u32(bytes, sectorOffset(holder) + (sector % entriesPerFatSector) * 4), + ).toBe(FREESECT); + } + }); + + it("chains every DIFAT sector correctly: each names a run of FAT sector indices then the next DIFAT sector or ENDOFCHAIN", () => { + const { bytes, fatSectorCount, difatSectorCount, difatStart } = + bigDifatFixture(); + const entriesPerFatSector = sectorSize / 4; + const difatEntriesPerSector = entriesPerFatSector - 1; + for (let sector = 0; sector < difatSectorCount; sector++) { + const base = sectorOffset(difatStart + sector); + for (let i = 0; i < difatEntriesPerSector; i++) { + const fatIndex = 109 + sector * difatEntriesPerSector + i; + if (fatIndex < fatSectorCount) { + expect(u32(bytes, base + i * 4)).toBe(fatIndex); + } else { + // Past the last real FAT sector, this slot is never written by the chaining loop below and must still read the FREESECT the DIFAT region's own initial fill leaves there -- this fixture's last DIFAT sector genuinely has such trailing slots, since 24 MiB does not divide evenly into whole DIFAT sectors of FAT-sector references. + expect(u32(bytes, base + i * 4)).toBe(FREESECT); + } + } + const terminator = u32(bytes, base + difatEntriesPerSector * 4); + if (sector === difatSectorCount - 1) { + expect(terminator).toBe(ENDOFCHAIN); + } else { + expect(terminator).toBe(difatStart + sector + 1); + } + } + }); + + it("needs no mini FAT sector at all when nothing is mini-resident", () => { + // This fixture's one stream is well past the mini-stream cutoff, so miniSectorCount is 0 and miniFatSectorCount (ceil(0 / 128)) must be 0 too -- a division-to-multiplication mutant on that same ceil would instead compute a large, clearly-wrong sector count from a genuinely zero numerator. + const { bytes, miniFatSectorCount } = bigDifatFixture(); + expect(miniFatSectorCount).toBe(0); + expect(u32(bytes, 0x3c)).toBe(ENDOFCHAIN); // first mini FAT sector: none needed + }); + + it("fills the DIFAT region's own reserved header array slots with FREESECT past the real FAT sector count", () => { + const { bytes, fatSectorCount } = bigDifatFixture(); + for (let i = fatSectorCount; i < 109; i++) { + expect(u32(bytes, 0x4c + i * 4)).toBe(FREESECT); + } + }); + + it("needs no chained DIFAT sector for exactly 109 FAT sectors, and exactly one past that", () => { + // 109 is HEADER_DIFAT_ENTRIES itself: the header's own array holds that many FAT sector locations unaided, so a file needing precisely 109 must not chain a DIFAT sector, while one needing 110 must chain exactly one. Payload sizes derived from the writer's own fixed-point sector-count loop to land exactly on each side of the boundary. + const atBoundary = writeCompoundFile([ + stream("A", new Uint8Array(7087104)), + ]); + expect(u32(atBoundary, 0x2c)).toBe(109); // fatSectorCount + expect(u32(atBoundary, 0x48)).toBe(0); // difatSectorCount + expect(u32(atBoundary, 0x44)).toBe(ENDOFCHAIN); // firstDifatSector: none needed + + const pastBoundary = writeCompoundFile([ + stream("A", new Uint8Array(7087616)), + ]); + expect(u32(pastBoundary, 0x2c)).toBe(110); + expect(u32(pastBoundary, 0x48)).toBe(1); + expect(u32(pastBoundary, 0x44)).not.toBe(ENDOFCHAIN); + }); }); diff --git a/packages/archive-codec/src/cfb/write.ts b/packages/archive-codec/src/cfb/write.ts index 6206e3cbf0..21ff386b8e 100644 --- a/packages/archive-codec/src/cfb/write.ts +++ b/packages/archive-codec/src/cfb/write.ts @@ -93,28 +93,28 @@ interface ResidentStream { readonly bytes: Uint8Array; } -// [MS-CFB] 2.6.4 uppercases one UTF-16 code point at a time using the simple (single-code-point) case mapping. JavaScript's toUpperCase applies the FULL mapping, which can expand one code unit into several ('ß' becomes 'SS'); wherever it does, the simple mapping is the identity, so an expansion means the code unit is left alone. Surrogates are never uppercased, because the spec's mapping is per code point and a surrogate is half of one. +// [MS-CFB] 2.6.4 uppercases one UTF-16 code point at a time using the simple (single-code-point) case mapping. JavaScript's toUpperCase applies the FULL mapping, which can expand one code unit into several ('ß' becomes 'SS'); wherever it does, the simple mapping is the identity, so an expansion means the code unit is left alone. No special-casing for a lone surrogate half (0xD800-0xDFFF): toUpperCase() already leaves every one of the 2048 surrogate code units completely unchanged (verified directly against every value in the range, not merely assumed), since none of them has a case mapping of its own, so the length-1 fallback below already returns exactly the same unit an explicit surrogate guard would. function upperCodeUnit(value: string, index: number): number { const unit = value.charCodeAt(index); - if (unit >= 0xd800 && unit <= 0xdfff) { - return unit; - } const upper = String.fromCharCode(unit).toUpperCase(); return upper.length === 1 ? upper.charCodeAt(0) : unit; } -// The [MS-CFB] 2.6.4 sorting relationship: a shorter name is less than a longer one, and equal-length names compare by uppercased UTF-16 code point. Length is compared as the code-unit count rather than the Directory Entry Name Length field the spec names, because that field is exactly (code units + 1) * 2 -- a strictly increasing function of the same quantity, so the two orderings are identical. Names that compare equal are the same name to the format, which is why this doubles as the sibling-uniqueness test. +// The [MS-CFB] 2.6.4 sorting relationship: a shorter name is less than a longer one, and equal-length names compare by uppercased UTF-16 code point. Length is compared as the code-unit count rather than the Directory Entry Name Length field the spec names, because that field is exactly (code units + 1) * 2 -- a strictly increasing function of the same quantity, so the two orderings are identical. Names that compare equal are the same name to the format, which is why this doubles as the sibling-uniqueness test. Walks left.split("") rather than a `for` loop bound by left.length: since both strings are already known equal-length here, an out-of-range comparison one iteration too long would compare charCodeAt(left.length) against itself on both sides (NaN against NaN, by construction identical), an equivalent mutant no input could ever distinguish -- split("") has no such bound to mismeasure in the first place, and .every's own short-circuit on returning false reproduces the early-return-on-first-difference behavior. function compareEntryNames(left: string, right: string): number { if (left.length !== right.length) { return left.length - right.length; } - for (let i = 0; i < left.length; i++) { + let result = 0; + left.split("").every((_unit, i) => { const difference = upperCodeUnit(left, i) - upperCodeUnit(right, i); if (difference !== 0) { - return difference; + result = difference; + return false; } - } - return 0; + return true; + }); + return result; } function checkedSegment(name: string, path: string): string { @@ -174,8 +174,8 @@ function addStream( } } -// The depth of the deepest node in the balanced tree linkSiblings builds over `count` siblings. Each recursion halves the sibling count, so the deepest node sits at floor(log2(count)) -- computed by bit length rather than Math.log2, which is a float operation whose rounding at exact powers of two would silently mis-colour a whole level. -function deepestDepth(count: number): number { +// The depth of the deepest node in the balanced tree linkSiblings builds over `count` siblings. Each recursion halves the sibling count, so the deepest node sits at floor(log2(count)) -- computed by bit length rather than Math.log2, which is a float operation whose rounding at exact powers of two would silently mis-colour a whole level. Exported for direct testing: its sole call site is deepestDepth(children.length), and when children.length is genuinely 0 (an empty storage, e.g. writeCompoundFile([])'s own root), linkSiblings returns undefined before ever reading the `deepest` argument at all -- so that one real call site can never observe whether count === 0 is handled correctly. +export function deepestDepth(count: number): number { return count === 0 ? 0 : 31 - Math.clz32(count); } @@ -251,6 +251,19 @@ function planDirectory(root: StorageNode): DirectoryPlan { return { rootPlan, plans }; } +// [MS-CFB] 2.6.1: a version 3 stream's size field has no high (>32-bit) half, so its byte length cannot exceed 0x80000000. Exported for direct testing against plain numbers: proving this boundary end to end would otherwise need constructing and writing an actual 2 GiB+ stream for every mutant of the condition itself, not merely the one real test that must still exist for the thrown message's own exact text. +export function exceedsVersion3StreamCeiling( + majorVersion: 3 | 4, + byteLength: number, +): boolean { + return majorVersion === 3 && byteLength > MAX_VERSION_3_STREAM_BYTES; +} + +// [MS-CFB] 2.6.1: the directory entry's stream-size field is a 64-bit little-endian quantity split across two 32-bit words; this is the high word (the low 32 bits, `size >>> 0`, need no such helper -- that operator has no other numeric reading a mutant could quietly substitute). Exported for direct testing against plain numbers for the same reason as exceedsVersion3StreamCeiling above: proving this arithmetic holds would otherwise need constructing and writing an actual 4 GiB+ stream. +export function highSizeWord(size: number): number { + return Math.floor(size / 4294967296); +} + // Writes the streams as a compound file. Version 3 (512-byte sectors) unless options say otherwise. Throws CompoundFileWriteError when the request itself cannot be expressed -- an illegal name, an empty path segment, colliding siblings, or a version 3 stream past the 2 GB the format allows one -- rather than emitting a file that only looks valid. export function writeCompoundFile( streams: readonly CompoundFileStream[], @@ -266,7 +279,7 @@ export function writeCompoundFile( const root: StorageNode = { name: ROOT_ENTRY_NAME, children: [] }; for (const { path, bytes } of streams) { - if (majorVersion === 3 && bytes.length > MAX_VERSION_3_STREAM_BYTES) { + if (exceedsVersion3StreamCeiling(majorVersion, bytes.length)) { throw new CompoundFileWriteError( `stream ${JSON.stringify(path)} is ${bytes.length} bytes, past the ${MAX_VERSION_3_STREAM_BYTES}-byte ceiling [MS-CFB] 2.6.1 puts on a version 3 stream; write the file as version 4 instead`, ); @@ -329,11 +342,13 @@ export function writeCompoundFile( entriesPerFatSector, ), ); - const neededDifat = - neededFat <= HEADER_DIFAT_ENTRIES - ? 0 - : Math.ceil((neededFat - HEADER_DIFAT_ENTRIES) / difatEntriesPerSector); - if (neededFat === fatSectorCount && neededDifat === difatSectorCount) { + // No neededFat <= HEADER_DIFAT_ENTRIES guard: HEADER_DIFAT_ENTRIES (109) is smaller than difatEntriesPerSector (127 for 512-byte sectors, 1023 for 4096-byte) for every sector size this writer supports, so whenever neededFat is genuinely at or under 109, (neededFat - HEADER_DIFAT_ENTRIES) is a negative number whose magnitude never reaches difatEntriesPerSector -- Math.ceil of that is always 0 (or -0, numerically identical) regardless, exactly the value the guard's own true branch spelled out a second time. Math.max(0, ...) makes that "never negative" invariant explicit rather than leaving it to a subtle cancellation between two magic numbers, and steers clear of -0 ever surfacing. + const neededDifat = Math.max( + 0, + Math.ceil((neededFat - HEADER_DIFAT_ENTRIES) / difatEntriesPerSector), + ); + // No `&& neededDifat === difatSectorCount` half to this check: difatSectorCount only ever gets set, a few lines below, to neededDifat computed from that same round's neededFat -- so difatSectorCount === g(fatSectorCount) is an invariant this loop maintains from its very first iteration (0 === g(1) initially, and every subsequent round re-establishes it by construction). The moment neededFat matches fatSectorCount, neededDifat = g(neededFat) = g(fatSectorCount), which by the invariant already equals difatSectorCount -- so the second comparison could never once observe a mismatch the first didn't already rule out. + if (neededFat === fatSectorCount) { break; } fatSectorCount = neededFat; @@ -401,11 +416,11 @@ export function writeCompoundFile( } }; - // The FAT describes its own sectors and the DIFAT's with role markers rather than chaining them ([MS-CFB] 2.3, 2.5); everything else is a chain. - for (let i = 0; i < fatSectorCount; i++) { + // The FAT describes its own sectors and the DIFAT's with role markers rather than chaining them ([MS-CFB] 2.3, 2.5); everything else is a chain. Both loops walk Array.from's own bounded index list rather than a hand-written comparison: an off-by-one here would mark one sector past its own region, but that sector is always the very first one the next region's own chain-writing call (the DIFAT loop below, or directoryStart's chainSectors when there is no DIFAT region at all) writes right afterwards -- so a stray extra iteration here is invisible in the finished file regardless, and removing the comparison removes the mutation opportunity along with it. + for (const i of Array.from({ length: fatSectorCount }, (_unused, n) => n)) { setFat(i, FATSECT); } - for (let i = 0; i < difatSectorCount; i++) { + for (const i of Array.from({ length: difatSectorCount }, (_unused, n) => n)) { setFat(difatStart + i, DIFSECT); } chainSectors(directoryStart, directorySectorCount); @@ -421,7 +436,11 @@ export function writeCompoundFile( } for (let sector = 0; sector < difatSectorCount; sector++) { const base = sectorOffset(difatStart + sector); - for (let i = 0; i < difatEntriesPerSector; i++) { + // Walks Array.from's own bounded index list rather than a hand-written comparison: an off-by-one running one slot past difatEntriesPerSector would write into the exact byte offset (base + difatEntriesPerSector * 4) the unconditional terminator write below writes to next, for this same sector -- so a stray extra iteration here is always overwritten immediately afterwards regardless, and removing the comparison removes the mutation opportunity along with it. + for (const i of Array.from( + { length: difatEntriesPerSector }, + (_unused, n) => n, + )) { const fatIndex = HEADER_DIFAT_ENTRIES + sector * difatEntriesPerSector + i; if (fatIndex < fatSectorCount) { @@ -473,7 +492,8 @@ export function writeCompoundFile( const base = entryOffset(entry.id); const node = entry.node; const name = node.name; - for (let i = 0; i < name.length; i++) { + // Walks Array.from's own bounded index list, by UTF-16 code unit (matching name.length, unlike code-point iteration which would miscount a surrogate pair) rather than a hand-written comparison: an off-by-one would write one code unit past the real name, but MAX_NAME_CODE_UNITS guarantees at least two zero bytes of gap remain there before the length field at 0x40 regardless, already zero from the allocation -- the extra write, charCodeAt() returning NaN past the string's own length and putU16 coercing that to 0 per DataView.setUint16's own ToUint16 semantics, changes nothing, so there is no comparison left here for a mutation to alter. + for (const i of Array.from({ length: name.length }, (_unused, n) => n)) { putU16(base + i * 2, name.charCodeAt(i)); } // The already-zero code unit past the name is the terminating null the length counts. @@ -486,7 +506,7 @@ export function writeCompoundFile( // CLSID (0x50), state bits (0x60), creation time (0x64), and modified time (0x6c) stay zero: [MS-CFB] 2.6.1 requires that of a stream entry and of the root's timestamps, and an implementation that does not let callers set a storage's class or state bits MUST default them to zero -- which is exactly this one, since none of it survives a round trip through the stream vocabulary this writer takes. putU32(base + 0x74, entry.startSector); putU32(base + 0x78, entry.size >>> 0); - putU32(base + 0x7c, Math.floor(entry.size / 4294967296)); + putU32(base + 0x7c, highSizeWord(entry.size)); } // Directory entries past the last real one pad their sector out. They stay object type 0 (unallocated) with a zero-length name, and only their links need writing, since NOSTREAM is not the zero the allocation already holds. for ( diff --git a/packages/archive-codec/src/crypto/md5.test.ts b/packages/archive-codec/src/crypto/md5.test.ts index 5fd37c0ada..cd1027ec93 100644 --- a/packages/archive-codec/src/crypto/md5.test.ts +++ b/packages/archive-codec/src/crypto/md5.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { md5 } from "./md5"; +import { md5, splitBitLength64, writeBitLength64 } from "./md5"; function toHex(bytes: Uint8Array): string { return Array.from(bytes) @@ -36,3 +36,38 @@ describe("md5", () => { expect(toHex(md5(input))).toBe("c1bb4f81d892b2d57947682aeb252456"); }); }); + +describe("splitBitLength64", () => { + it("keeps the whole value in the low half when it fits in 32 bits", () => { + expect(splitBitLength64(640)).toEqual({ low: 640, high: 0 }); + }); + + it("carries the excess into the high half once the bit length passes 2^32", () => { + // A message longer than 512 MiB overflows the low 32-bit half -- exercised here directly against a fabricated bit length, since hashing an actual 512 MiB buffer to reach this boundary would make the suite itself pathologically slow. + expect(splitBitLength64(0x100000005)).toEqual({ low: 5, high: 1 }); + }); + + it("keeps splitting correctly for a bit length spanning several high-half units", () => { + expect(splitBitLength64(0x300000010)).toEqual({ low: 0x10, high: 3 }); + }); +}); + +describe("writeBitLength64", () => { + it("writes both halves little-endian, including a non-zero high half", () => { + // A bit length whose high half is non-zero and distinct from its low half, written directly rather than via an actual >512 MiB message -- proving the high half's own byte order without hashing anything pathologically large. A big-endian mistake on the high half would write 0x02000000 here, not 0x00000002. + const buffer = new ArrayBuffer(8); + const view = new DataView(buffer); + writeBitLength64(view, 0, 0x200000005); + expect(view.getUint32(0, true)).toBe(5); + expect(view.getUint32(4, true)).toBe(2); + expect([...new Uint8Array(buffer)]).toEqual([5, 0, 0, 0, 2, 0, 0, 0]); + }); + + it("writes at a non-zero offset", () => { + const buffer = new ArrayBuffer(10); + const view = new DataView(buffer); + writeBitLength64(view, 2, 640); + expect(view.getUint32(2, true)).toBe(640); + expect(view.getUint32(6, true)).toBe(0); + }); +}); diff --git a/packages/archive-codec/src/crypto/md5.ts b/packages/archive-codec/src/crypto/md5.ts index 13d97efa5d..7a03dd90d1 100644 --- a/packages/archive-codec/src/crypto/md5.ts +++ b/packages/archive-codec/src/crypto/md5.ts @@ -50,7 +50,29 @@ function rotl32(value: number, bits: number): number { return ((value << bits) | (value >>> (32 - bits))) >>> 0; } -// RFC 1321 3.1/3.2: append 0x80, then zero bytes until the length is 56 mod 64, then the original *bit* length as a 64-bit little-endian integer. The length is split into two 32-bit halves by ordinary arithmetic rather than shifts, since a message longer than 512 MB overflows a 32-bit bit-count while staying exactly representable as a JS number. +// RFC 1321 3.1: the original *bit* length as a 64-bit quantity, split into two 32-bit halves by ordinary arithmetic rather than shifts, since a message longer than 512 MiB overflows a 32-bit bit-count while staying exactly representable as a JS number. Exported (rather than kept as padMessage's own local arithmetic) so the >2^32 boundary -- reached only by a message past 512 MiB -- is directly testable: hashing an actual 512 MiB buffer through this hand-written implementation to exercise it indirectly would make the test suite itself pathologically slow. +export function splitBitLength64(bitLength: number): { + readonly low: number; + readonly high: number; +} { + return { + low: bitLength % 0x100000000, + high: Math.floor(bitLength / 0x100000000), + }; +} + +// RFC 1321 3.1/3.2's own 64-bit little-endian bit-length field, as its two 32-bit halves. A dedicated function (rather than padMessage's own inline pair of writes) so the high half's own byte order is directly testable: high is non-zero only for a message past 512 MiB, and hashing an actual buffer that large just to reach it through padMessage would make the test suite itself pathologically slow. +export function writeBitLength64( + view: DataView, + offset: number, + bitLength: number, +): void { + const { low, high } = splitBitLength64(bitLength); + view.setUint32(offset, low, true); + view.setUint32(offset + 4, high, true); +} + +// RFC 1321 3.1/3.2: append 0x80, then zero bytes until the length is 56 mod 64, then the original bit length as a 64-bit little-endian integer (DataView's own setUint32 handles the byte order, rather than a hand-rolled per-byte shift-and-mask loop). function padMessage(bytes: Uint8Array): Uint8Array { const paddedLength = (Math.floor((bytes.length + 8) / BLOCK_BYTES) + 1) * BLOCK_BYTES; @@ -58,15 +80,7 @@ function padMessage(bytes: Uint8Array): Uint8Array { padded.set(bytes); padded[bytes.length] = 0x80; const view = new DataView(padded.buffer); - const bitLength = bytes.length * 8; - let low = bitLength % 0x100000000; - let high = Math.floor(bitLength / 0x100000000); - for (let i = 0; i < 4; i++) { - view.setUint8(paddedLength - 8 + i, low & 0xff); - low = Math.floor(low / 256); - view.setUint8(paddedLength - 4 + i, high & 0xff); - high = Math.floor(high / 256); - } + writeBitLength64(view, paddedLength - 8, bytes.length * 8); return padded; } diff --git a/packages/archive-codec/src/crypto/office-rc4-cryptoapi.test.ts b/packages/archive-codec/src/crypto/office-rc4-cryptoapi.test.ts index f675e0bb50..c0b67221c8 100644 --- a/packages/archive-codec/src/crypto/office-rc4-cryptoapi.test.ts +++ b/packages/archive-codec/src/crypto/office-rc4-cryptoapi.test.ts @@ -4,6 +4,7 @@ import { verifyRc4CryptoApiPassword, } from "./office-rc4-cryptoapi"; import { rc4 } from "./rc4"; +import { sha1 } from "./sha1"; function toHex(bytes: Uint8Array): string { return Array.from(bytes) @@ -82,6 +83,29 @@ describe("verifyRc4CryptoApiPassword", () => { ), ).toBe(false); }); + + it("rejects a decrypted hash that shares one byte with the verifier's own SHA-1 but otherwise disagrees, not just a wholesale mismatch", () => { + // A verifier/hash pair built to share exactly one byte (index 0) at whatever position a byte-by-byte comparison checks first, with every other byte of the hash deliberately complemented (`0xff - b` can never equal `b`, so every other position is guaranteed to differ) -- this is exactly the input that would fool a comparison using `.some` (true the moment any single byte matches) where only `.every` (true only when every byte matches) is correct. + const verifier = new Uint8Array(16).fill(0x42); + const correctHash = sha1(verifier); + const tamperedHash = correctHash.map((byte, index) => + index === 0 ? byte : 0xff - byte, + ); + const key = deriveRc4CryptoApiBlockKey(PASSWORD, SALT, 0, 128); + const combined = new Uint8Array(36); + combined.set(verifier, 0); + combined.set(tamperedHash, 16); + const ciphertext = rc4(key, combined); + expect( + verifyRc4CryptoApiPassword( + PASSWORD, + SALT, + 128, + ciphertext.subarray(0, 16), + ciphertext.subarray(16, 36), + ), + ).toBe(false); + }); }); describe("deriveRc4CryptoApiBlockKey combined with this package's own rc4", () => { diff --git a/packages/archive-codec/src/crypto/office-rc4-cryptoapi.ts b/packages/archive-codec/src/crypto/office-rc4-cryptoapi.ts index d74957bcce..30f06a88b0 100644 --- a/packages/archive-codec/src/crypto/office-rc4-cryptoapi.ts +++ b/packages/archive-codec/src/crypto/office-rc4-cryptoapi.ts @@ -19,13 +19,11 @@ function keySizeBitsOf(keySizeBits: number): number { return keySizeBits === 0 ? RC4_CRYPTOAPI_DEFAULT_KEY_SIZE_BITS : keySizeBits; } +// Constant-shape comparison, deliberately with no length check: this module's own sole call site (verifyRc4CryptoApiPassword below) always compares a SHA-1 digest (always 20 bytes) against a slice already fixed to that same RC4_CRYPTOAPI_VERIFIER_HASH_LENGTH -- a mismatched-length pair can never actually reach this private helper, so guarding against one would be dead code for a case this module cannot produce. function bytesEqual( a: Uint8Array, b: Uint8Array, ): boolean { - if (a.length !== b.length) { - return false; - } return a.every((byte, index) => byte === b[index]); } diff --git a/packages/archive-codec/src/crypto/office-rc4.ts b/packages/archive-codec/src/crypto/office-rc4.ts index cd4aa03ef5..c3f0f1ee35 100644 --- a/packages/archive-codec/src/crypto/office-rc4.ts +++ b/packages/archive-codec/src/crypto/office-rc4.ts @@ -19,10 +19,10 @@ export function passwordToUtf16LeBytes( password: string, ): Uint8Array { const bytes = new Uint8Array(password.length * 2); + // A DataView write, not raw indexed assignment: an out-of-range DataView offset throws, where a plain `bytes[i] = …` past the array's own end silently does nothing -- so a loop bound one iteration too long fails loudly here instead of leaving the same, indistinguishable output. + const view = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength); for (let i = 0; i < password.length; i += 1) { - const code = password.charCodeAt(i); - bytes[i * 2] = code & 0xff; - bytes[i * 2 + 1] = (code >>> 8) & 0xff; + view.setUint16(i * 2, password.charCodeAt(i), true); } return bytes; } diff --git a/packages/archive-codec/src/crypto/sha1.test.ts b/packages/archive-codec/src/crypto/sha1.test.ts index d5c4c014b4..1babcd13ce 100644 --- a/packages/archive-codec/src/crypto/sha1.test.ts +++ b/packages/archive-codec/src/crypto/sha1.test.ts @@ -29,9 +29,9 @@ describe("sha1", () => { expect(toHex(sha1(input))).toBe("bb2fa3ee7afb9f54c6dfb5d021f14b1ffe40c163"); }); - // RFC 3174 7.3's own third vector: one million repetitions of "a". Heavier than the others deliberately -- it is the one vector in the published suite that exercises the multi-block message schedule across many blocks rather than just one or two. + // RFC 3174 7.3's own third vector: one million repetitions of "a". Heavier than the others deliberately -- it is the one vector in the published suite that exercises the multi-block message schedule across many blocks rather than just one or two. Given an explicit, generous timeout (rather than vitest's 5000ms default) because per-test coverage instrumentation (Stryker's dry run, or plain `--coverage`) measurably multiplies this test's own real cost well past that default on a loaded machine, independent of anything this suite is actually testing. it("hashes one million repetitions of 'a'", () => { const input = ascii("a".repeat(1_000_000)); expect(toHex(sha1(input))).toBe("34aa973cd4c4daa4f61eeb2bdbad27316534016f"); - }); + }, 30_000); }); diff --git a/packages/archive-codec/src/crypto/xor-obfuscation.test.ts b/packages/archive-codec/src/crypto/xor-obfuscation.test.ts index 4bc9130d98..24b6ba241b 100644 --- a/packages/archive-codec/src/crypto/xor-obfuscation.test.ts +++ b/packages/archive-codec/src/crypto/xor-obfuscation.test.ts @@ -40,19 +40,25 @@ describe("createXorObfuscationKey / createXorObfuscationPasswordVerifier", () => }); it("rejects a password longer than 15 characters", () => { + // The exact message, not just the error class: a downstream out-of-range table lookup a few lines later also throws a RangeError for a 16-character password, so an assertion on the error class alone would not actually prove this guard fired. expect(() => createXorObfuscationKey("1234567890123456")).toThrow( - RangeError, + "XOR obfuscation passwords must be 1-15 characters, got 16", ); }); it("rejects an empty password", () => { - expect(() => createXorObfuscationKey("")).toThrow(RangeError); + expect(() => createXorObfuscationKey("")).toThrow( + "XOR obfuscation passwords must be 1-15 characters, got 0", + ); }); it("rejects a password with a character outside single-byte ASCII/Latin-1", () => { expect(() => createXorObfuscationKey("pässwörd")).not.toThrow(); + // U+00FF is the highest single-byte Latin-1 code point this module accepts -- the exact boundary the ASCII/Latin-1 check must get right. + expect(() => createXorObfuscationKey("ÿ")).not.toThrow(); + // charCodeAt reads UTF-16 code units, so the emoji's own high surrogate (0xD83D = 55357) is what surfaces at index 4, not its full code point. expect(() => createXorObfuscationKey("pass\u{1F600}word")).toThrow( - RangeError, + "XOR obfuscation passwords must be single-byte ASCII/Latin-1 characters, got code point 55357 at index 4", ); }); }); @@ -175,4 +181,17 @@ describe("decryptXorObfuscationMethod2", () => { const decrypted = decryptXorObfuscationMethod2(array, data, 0); expect(decrypted).toEqual(data); }); + + it("leaves a non-zero byte unmodified when XORing it against the array happens to produce zero", () => { + // [MS-OFFCRYPTO] 2.3.7.6's own zero-exception applies whenever EITHER the original byte or the transformed result is zero, not only when the input already was -- a byte that XORs to zero against its own array entry (transformed === 0) must stay as its own original, non-zero value, exactly as a genuinely zero input byte does. + const array = createXorObfuscationArray( + "Test1234", + XOR_OBFUSCATION_ROTATE_DISTANCE_METHOD2, + ); + const firstArrayByte = array[0] ?? 0; + expect(firstArrayByte).not.toBe(0); + const data = new Uint8Array([firstArrayByte]); + const decrypted = decryptXorObfuscationMethod2(array, data, 0); + expect(decrypted).toEqual(data); + }); }); diff --git a/packages/archive-codec/src/crypto/xor-obfuscation.ts b/packages/archive-codec/src/crypto/xor-obfuscation.ts index 31ae593567..b2af71c9c7 100644 --- a/packages/archive-codec/src/crypto/xor-obfuscation.ts +++ b/packages/archive-codec/src/crypto/xor-obfuscation.ts @@ -68,6 +68,12 @@ function passwordToAsciiBytes(password: string): Uint8Array { ); } const bytes = new Uint8Array(password.length); + // A DataView write, not raw indexed assignment: an out-of-range DataView offset throws, where a plain `bytes[i] = …` past the array's own end silently does nothing -- so a loop bound one iteration too long fails loudly here instead of leaving the same, indistinguishable output. + const bytesView = new DataView( + bytes.buffer, + bytes.byteOffset, + bytes.byteLength, + ); for (let i = 0; i < password.length; i += 1) { const code = password.charCodeAt(i); if (code > 0xff) { @@ -75,7 +81,7 @@ function passwordToAsciiBytes(password: string): Uint8Array { `XOR obfuscation passwords must be single-byte ASCII/Latin-1 characters, got code point ${code} at index ${i}`, ); } - bytes[i] = code; + bytesView.setUint8(i, code); } return bytes; } @@ -137,18 +143,19 @@ export function createXorObfuscationArray( ): Uint8Array { const passwordBytes = passwordToAsciiBytes(password); const array = new Uint8Array(XOR_OBFUSCATION_ARRAY_LENGTH); + const arrayView = new DataView( + array.buffer, + array.byteOffset, + array.byteLength, + ); array.set(passwordBytes, 0); + // A DataView write for the padding fill, not raw indexed assignment: an out-of-range DataView offset throws, where a plain `array[i] = …` past the array's own 16-byte end silently does nothing -- so a loop bound one iteration too long fails loudly here instead of leaving the same, indistinguishable 16-byte array. for (let i = passwordBytes.length; i < XOR_OBFUSCATION_ARRAY_LENGTH; i += 1) { - array[i] = PAD_ARRAY.getUint8(i - passwordBytes.length); + arrayView.setUint8(i, PAD_ARRAY.getUint8(i - passwordBytes.length)); } const xorKey = createXorObfuscationKey(password); const keyLow = xorKey & 0xff; const keyHigh = (xorKey >>> 8) & 0xff; - const arrayView = new DataView( - array.buffer, - array.byteOffset, - array.byteLength, - ); for (let i = 0; i < XOR_OBFUSCATION_ARRAY_LENGTH; i += 1) { const withKey = arrayView.getUint8(i) ^ (i % 2 === 0 ? keyLow : keyHigh); array[i] = rotateLeft8(withKey, rotateDistance); diff --git a/packages/archive-codec/src/magic.ts b/packages/archive-codec/src/magic.ts index 4e3ed74ef2..29c0836023 100644 --- a/packages/archive-codec/src/magic.ts +++ b/packages/archive-codec/src/magic.ts @@ -1,9 +1,8 @@ -// Shared leading-bytes magic check for the format-detection modules: true when bytes begins with magic byte-for-byte. Deliberately not exported from the barrel -- it is an internal helper of the detectors, not package surface. +// Shared leading-bytes magic check for the format-detection modules: true when bytes begins with magic byte-for-byte. Deliberately not exported from the barrel -- it is an internal helper of the detectors, not package surface. No separate length pre-check: indexing a Uint8Array past its own end reads back `undefined`, which can never equal one of `magic`'s numeric entries, so a `bytes` shorter than `magic` already falls out of the loop as false on its own. export function startsWithMagic( bytes: Uint8Array, magic: readonly number[], ): boolean { - if (bytes.length < magic.length) return false; for (let i = 0; i < magic.length; i++) { if (bytes[i] !== magic[i]) return false; } diff --git a/packages/archive-codec/src/oleps/read.test.ts b/packages/archive-codec/src/oleps/read.test.ts index 4173756620..3dbb8ceafa 100644 --- a/packages/archive-codec/src/oleps/read.test.ts +++ b/packages/archive-codec/src/oleps/read.test.ts @@ -1,11 +1,32 @@ import { describe, expect, it } from "vitest"; import { propertySetStream } from "../test-support/oleps"; -import { PropertySetFormatError, readPropertySetStream } from "./read"; +import { + PropertySetFormatError, + decodeCodepage, + readPropertySetStream, + truncateAtNull, +} from "./read"; +import { + HEADER_SIZE, + IDENTIFIER_AND_OFFSET_SIZE, + PROPERTY_SET_HEADER_SIZE, + TYPED_VALUE_HEADER_SIZE, +} from "./wire"; // Coverage for the generic [MS-OLEPS] Property Set Stream reader (src/oleps/read.ts). The primary fixture below is transcribed byte-for-byte from [MS-OLEPS]'s own worked "SummaryInformation Property Set" example (the stream contents table in the spec's SummaryInformation Property Set section) -- the strongest possible validation, since it proves this reader parses a real, complete, unmodified 444-byte stream a genuine implementation produced, not merely bytes this reader's own writer happens to agree with itself about. It exercises every property type this reader supports (VT_I2 for CodePage, VT_LPSTR for every string property, VT_FILETIME for every timestamp, VT_I4 for every count) in one pass. Additional fixtures below it, built via ../test-support/oleps.ts (independent of ./write.ts's own construction), cover round-trip correctness for values this reader's own numbers can be hand-verified against, and the structural error paths. const FMTID_SUMMARY_INFORMATION = "{F29F85E0-4FF9-1068-AB91-08002B27B3D9}"; +// propertySetStream always writes Offset0 = HEADER_SIZE and a genuine PropertySet packet Size covering everything it built, so truncating the array alone to test a check deeper than that declared Size always trips the Size check itself first (it runs earlier in readPropertySetStream, and any truncation short of the packet's true end is also short of its declared Size, which describes that same true end). Corrupting the Size field down to 0 -- nothing else in this reader ever reads it again -- lets the array's own physical truncation reach a boundary further in without the Size check intercepting it. +function truncatedPastDeclaredSize( + full: Uint8Array, + length: number, +): Uint8Array { + const bytes = full.subarray(0, length); + new DataView(bytes.buffer).setUint32(HEADER_SIZE, 0, true); + return bytes; +} + // prettier-ignore const SUMMARY_INFORMATION_WORKED_EXAMPLE = new Uint8Array([ // 00x @@ -249,8 +270,8 @@ describe("readPropertySetStream", () => { ]); const view = new DataView(bytes.buffer); // PID 0x11 is the dictionary's second entry (index 1); read its own relativeOffset back out rather than hand-deriving the byte length of PID 2's preceding VT_LPWSTR value. - const HEADER_SIZE = 48; - const dictionaryEntryOffset = HEADER_SIZE + 8 + 1 * 8; + const dictionaryEntryOffset = + HEADER_SIZE + PROPERTY_SET_HEADER_SIZE + 1 * IDENTIFIER_AND_OFFSET_SIZE; const relativeOffset = view.getUint32(dictionaryEntryOffset + 4, true); view.setUint16(HEADER_SIZE + relativeOffset, 0x0047, true); // VT_CF const propertySet = readPropertySetStream(bytes); @@ -265,9 +286,282 @@ describe("readPropertySetStream", () => { expect(propertySet.properties.has(0x11)).toBe(false); }); - it("throws PropertySetFormatError when a stream is shorter than the fixed header", () => { - expect(() => readPropertySetStream(new Uint8Array(10))).toThrow( - PropertySetFormatError, + it("throws naming the PropertySetStream header's own required size when the stream is shorter than it", () => { + const bytes = new Uint8Array(HEADER_SIZE - 1); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before the PropertySetStream header (needs ${HEADER_SIZE} bytes at offset 0, stream is ${HEADER_SIZE - 1} bytes)`, + ); + }); + + it("throws naming the exact ByteOrder value found, in hex, for a ByteOrder field other than 0xFFFE", () => { + const bytes = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_LPWSTR", value: "x" } }, + ]); + bytes.set([0x34, 0x12], 0); // 0x1234 little-endian -- a value distinguishable from its own byte-swapped reading, unlike an all-zero or palindromic one + expect(() => readPropertySetStream(bytes)).toThrow( + "property set stream's ByteOrder field is 0x1234, not the mandated 0xFFFE", + ); + }); + + it("names its own error class PropertySetFormatError, not merely an instance of it", () => { + const bytes = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_LPWSTR", value: "x" } }, + ]); + bytes.set([0x34, 0x12], 0); + let caught: unknown; + try { + readPropertySetStream(bytes); + } catch (error) { + caught = error; + } + expect((caught as Error).name).toBe("PropertySetFormatError"); + }); + + it("throws naming the exact declared count for NumPropertySets other than 1", () => { + const bytes = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_LPWSTR", value: "x" } }, + ]); + const view = new DataView(bytes.buffer); + view.setUint32(24, 2, true); + expect(() => readPropertySetStream(bytes)).toThrow( + 'property set stream declares 2 property sets; this reader only handles the single-property-set form every "\\x05SummaryInformation" stream uses (the two-property-set DocumentSummaryInformation/UserDefinedProperties spelling is out of scope, see the package README)', + ); + }); + + it("throws its exact message for a Dictionary property (PID 0)", () => { + const bytes = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 0, value: { type: "VT_LPWSTR", value: "x" } }, + ]); + expect(() => readPropertySetStream(bytes)).toThrow( + 'property set carries a Dictionary property (PID 0), which names string-keyed properties this reader does not support -- no "\\x05SummaryInformation" stream should carry one', + ); + }); + + it("throws naming the PropertySet packet header's own required size when the packet header doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_I4", value: 1 } }, + ]); + const truncatedLength = HEADER_SIZE + PROPERTY_SET_HEADER_SIZE - 1; + const bytes = full.subarray(0, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before the PropertySet packet header (needs ${PROPERTY_SET_HEADER_SIZE} bytes at offset ${HEADER_SIZE}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming the PropertySet packet's own declared Size when the stream ends before it", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_I4", value: 1 } }, + ]); + // The stream's total length is exactly offset0 + the declared Size (HEADER_SIZE, since propertySetStream always writes Offset0 as HEADER_SIZE, plus the PropertySet packet's own bytes) -- one byte short of that is one byte short of the declared Size fitting. + const truncatedLength = full.length - 1; + const bytes = full.subarray(0, truncatedLength); + const declaredSize = new DataView(full.buffer).getUint32(HEADER_SIZE, true); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before the PropertySet packet's own declared Size (needs ${declaredSize} bytes at offset ${HEADER_SIZE}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming the PropertyIdentifierAndOffset dictionary's own required size when it doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_I4", value: 1 } }, + ]); + const tableStart = HEADER_SIZE + PROPERTY_SET_HEADER_SIZE; + const dictionaryLength = 1 * IDENTIFIER_AND_OFFSET_SIZE; + const truncatedLength = tableStart + dictionaryLength - 1; + const bytes = truncatedPastDeclaredSize(full, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before the PropertyIdentifierAndOffset dictionary (needs ${dictionaryLength} bytes at offset ${tableStart}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming the CodePage property's own TypedPropertyValue size when it doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 1, value: { type: "VT_I2", value: 1252 } }, + ]); + const abs = + HEADER_SIZE + PROPERTY_SET_HEADER_SIZE + IDENTIFIER_AND_OFFSET_SIZE; + const requiredLength = TYPED_VALUE_HEADER_SIZE + 4; + const truncatedLength = abs + requiredLength - 1; + const bytes = truncatedPastDeclaredSize(full, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before the CodePage property's TypedPropertyValue (needs ${requiredLength} bytes at offset ${abs}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming the exact type code found, in hex, when the CodePage property is not VT_I2", () => { + const bytes = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 1, value: { type: "VT_I4", value: 1252 } }, + ]); + expect(() => readPropertySetStream(bytes)).toThrow( + "CodePage property (PID 1) has type 0x3, not VT_I2 as [MS-OLEPS] requires", + ); + }); + + it("throws naming a property's own TypedPropertyValue header size when it doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_I4", value: 1 } }, + ]); + const abs = + HEADER_SIZE + PROPERTY_SET_HEADER_SIZE + IDENTIFIER_AND_OFFSET_SIZE; + const truncatedLength = abs + TYPED_VALUE_HEADER_SIZE - 1; + const bytes = truncatedPastDeclaredSize(full, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before a property's TypedPropertyValue header (needs ${TYPED_VALUE_HEADER_SIZE} bytes at offset ${abs}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming the exact padding value found, in hex, decoded little-endian, when TypedPropertyValue padding is non-zero", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_I4", value: 1 } }, + ]); + const abs = + HEADER_SIZE + PROPERTY_SET_HEADER_SIZE + IDENTIFIER_AND_OFFSET_SIZE; + // Two distinct, non-zero bytes: little-endian and big-endian readings of them produce different numbers (0x3412 vs 0x1234), so asserting the exact hex value in the message proves the padding is decoded in the byte order [MS-OLEPS] uses everywhere else, not merely checked against zero. + const bytes = full.subarray(); + new DataView(bytes.buffer).setUint16(abs + 2, 0x1234, true); + expect(() => readPropertySetStream(bytes)).toThrow( + "property 2's TypedPropertyValue padding is 0x1234, not zero as [MS-OLEPS] requires", + ); + }); + + it("throws naming the VT_I2 value's own required size when it doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_I2", value: 1 } }, + ]); + const valueOffset = + HEADER_SIZE + + PROPERTY_SET_HEADER_SIZE + + IDENTIFIER_AND_OFFSET_SIZE + + TYPED_VALUE_HEADER_SIZE; + const truncatedLength = valueOffset + 4 - 1; + const bytes = truncatedPastDeclaredSize(full, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before property 2's VT_I2 value (needs 4 bytes at offset ${valueOffset}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming the VT_I4 value's own required size when it doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_I4", value: 1 } }, + ]); + const valueOffset = + HEADER_SIZE + + PROPERTY_SET_HEADER_SIZE + + IDENTIFIER_AND_OFFSET_SIZE + + TYPED_VALUE_HEADER_SIZE; + const truncatedLength = valueOffset + 4 - 1; + const bytes = truncatedPastDeclaredSize(full, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before property 2's VT_I4 value (needs 4 bytes at offset ${valueOffset}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming the VT_FILETIME value's own required size when it doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 12, value: { type: "VT_FILETIME", low: 0, high: 0 } }, + ]); + const valueOffset = + HEADER_SIZE + + PROPERTY_SET_HEADER_SIZE + + IDENTIFIER_AND_OFFSET_SIZE + + TYPED_VALUE_HEADER_SIZE; + const truncatedLength = valueOffset + 8 - 1; + const bytes = truncatedPastDeclaredSize(full, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before property 12's VT_FILETIME value (needs 8 bytes at offset ${valueOffset}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming a CodePageString's own Size field size when it doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_LPSTR", value: "x" } }, + ]); + const valueOffset = + HEADER_SIZE + + PROPERTY_SET_HEADER_SIZE + + IDENTIFIER_AND_OFFSET_SIZE + + TYPED_VALUE_HEADER_SIZE; + const truncatedLength = valueOffset + 4 - 1; + const bytes = truncatedPastDeclaredSize(full, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before a CodePageString's Size field (needs 4 bytes at offset ${valueOffset}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming a CodePageString's own Characters field size when it doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_LPSTR", value: "x" } }, // Size = "x".length + 1 = 2 + ]); + const valueOffset = + HEADER_SIZE + + PROPERTY_SET_HEADER_SIZE + + IDENTIFIER_AND_OFFSET_SIZE + + TYPED_VALUE_HEADER_SIZE; + const charactersOffset = valueOffset + 4; + const truncatedLength = charactersOffset + 2 - 1; + const bytes = truncatedPastDeclaredSize(full, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before a CodePageString's Characters field (needs 2 bytes at offset ${charactersOffset}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming a UnicodeString's own Length field size when it doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_LPWSTR", value: "x" } }, + ]); + const valueOffset = + HEADER_SIZE + + PROPERTY_SET_HEADER_SIZE + + IDENTIFIER_AND_OFFSET_SIZE + + TYPED_VALUE_HEADER_SIZE; + const truncatedLength = valueOffset + 4 - 1; + const bytes = truncatedPastDeclaredSize(full, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before a UnicodeString's Length field (needs 4 bytes at offset ${valueOffset}, stream is ${truncatedLength} bytes)`, + ); + }); + + it("throws naming a UnicodeString's own Characters field size when it doesn't fit", () => { + const full = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 2, value: { type: "VT_LPWSTR", value: "x" } }, // Length (16-bit units) = "x".length + 1 = 2, so Characters is 4 bytes + ]); + const valueOffset = + HEADER_SIZE + + PROPERTY_SET_HEADER_SIZE + + IDENTIFIER_AND_OFFSET_SIZE + + TYPED_VALUE_HEADER_SIZE; + const charactersOffset = valueOffset + 4; + const truncatedLength = charactersOffset + 4 - 1; + const bytes = truncatedPastDeclaredSize(full, truncatedLength); + expect(() => readPropertySetStream(bytes)).toThrow( + `property set stream ends before a UnicodeString's Characters field (needs 4 bytes at offset ${charactersOffset}, stream is ${truncatedLength} bytes)`, ); }); }); + +describe("decodeCodepage", () => { + it("returns a non-negative raw value unchanged, including exactly zero", () => { + expect(decodeCodepage(0)).toBe(0); + expect(decodeCodepage(1252)).toBe(1252); + expect(decodeCodepage(32767)).toBe(32767); + }); + + it("adds 0x10000 to a negative raw value, recovering the unsigned codepage a real producer declared", () => { + expect(decodeCodepage(-1)).toBe(0xffff); + expect(decodeCodepage(-32768)).toBe(0x8000); + }); +}); + +describe("truncateAtNull", () => { + it("returns the value unchanged when it carries no null code unit at all", () => { + expect(truncateAtNull("no null here")).toBe("no null here"); + }); + + it("returns an empty string when the null is the very first code unit", () => { + expect(truncateAtNull("trailing garbage")).toBe(""); + }); + + it("truncates at a null code unit in the middle, dropping everything from it onward", () => { + expect(truncateAtNull("keptdropped")).toBe("kept"); + }); +}); diff --git a/packages/archive-codec/src/oleps/read.ts b/packages/archive-codec/src/oleps/read.ts index eb8368ad20..3397efa2e4 100644 --- a/packages/archive-codec/src/oleps/read.ts +++ b/packages/archive-codec/src/oleps/read.ts @@ -30,19 +30,25 @@ export class PropertySetFormatError extends Error { } } +// No offset<0/length<0 guard: every call site below derives both from a getUint32/getInt16 read (always non-negative) or a positive literal/constant, so neither can ever actually be negative -- a defensive check against an input this module never produces. function requireBytes( byteLength: number, offset: number, length: number, what: string, ): void { - if (offset < 0 || length < 0 || offset + length > byteLength) { + if (offset + length > byteLength) { throw new PropertySetFormatError( `property set stream ends before ${what} (needs ${length} bytes at offset ${offset}, stream is ${byteLength} bytes)`, ); } } +// VT_I2's own Value field is a signed 16-bit integer, but a codepage above 32767 is conventionally stored as its negative two's-complement equivalent -- this undoes that back to the unsigned codepage number a real producer declared. Exported for direct testing: the resulting number is only ever compared against CP_WINUNICODE/WINDOWS_1252_CODEPAGE downstream, neither of which a boundary mistake at raw === 0 would ever produce either way, so no decoding outcome could otherwise distinguish the two. +export function decodeCodepage(raw: number): number { + return raw < 0 ? raw + 0x10000 : raw; +} + const ANSI_DECODER = new TextDecoder("windows-1252"); const UTF16_DECODER = new TextDecoder("utf-16le"); @@ -58,7 +64,8 @@ function decodeAnsi( } // [MS-OLEPS] 2.19/2.20: both string packets MAY carry embedded or additional trailing null characters beyond the first terminator, and how a reader "presents" such a string to its application is implementation-specific. This one truncates at the first null code unit -- what every string ./write.ts and ./summary-information.ts actually produce needs (a plain string, no embedded nulls), and what the spec's own worked SummaryInformation example requires to read an empty property back as "" rather than as embedded NUL characters (its KEYWORDS property is four zero bytes: Size 4, not the minimal Size 1 a null-terminator-only empty string would use). -function truncateAtNull(value: string): string { +// Exported for direct testing: every fixture this module's own tests decode already carries a real producer's null terminator, so a round trip alone never exercises the "no null present at all" branch. +export function truncateAtNull(value: string): string { const index = value.indexOf("\u0000"); return index === -1 ? value : value.slice(0, index); } @@ -187,8 +194,7 @@ export function readPropertySetStream( ); } const raw = view.getInt16(abs + TYPED_VALUE_HEADER_SIZE, true); - // Codepages above 32767 are conventionally stored as their negative 16-bit twos-complement equivalent, since VT_I2's own Value is a signed integer. - codepage = raw < 0 ? raw + 0x10000 : raw; + codepage = decodeCodepage(raw); } const properties = new Map(); @@ -208,6 +214,7 @@ export function readPropertySetStream( ); } const valueOffset = abs + TYPED_VALUE_HEADER_SIZE; + // A PropertyType this switch names no case for (e.g. VT_CF, a PIDSI_THUMBNAIL clipboard format) is skipped rather than aborting the whole read, since an undecodable value is a projection gap, not a structural violation (see the module comment above). No explicit default case: falling out of a switch with no matching case is already exactly that -- a no-op -- so a default whose own body is just `break` would only restate what already happens, and its break is otherwise dead code every case above it already itself carries. switch (type) { case VT_I2: { requireBytes( @@ -263,9 +270,6 @@ export function readPropertySetStream( }); break; } - default: - // A PropertyType this reader does not decode (e.g. VT_CF, a PIDSI_THUMBNAIL clipboard format) -- skipped rather than aborting the whole read, since an undecodable value is a projection gap, not a structural violation (see the module comment above). - break; } } diff --git a/packages/archive-codec/src/oleps/summary-information.test.ts b/packages/archive-codec/src/oleps/summary-information.test.ts index 971c2821b9..23f0f65949 100644 --- a/packages/archive-codec/src/oleps/summary-information.test.ts +++ b/packages/archive-codec/src/oleps/summary-information.test.ts @@ -1,11 +1,12 @@ import { describe, expect, it } from "vitest"; -import { PropertySetFormatError, readPropertySetStream } from "./read"; +import { readPropertySetStream } from "./read"; import { FMTID_SUMMARY_INFORMATION, readSummaryInformation, writeSummaryInformationStream, } from "./summary-information"; import { writePropertySetStream } from "./write"; +import { propertySetStream } from "../test-support/oleps"; describe("readSummaryInformation / writeSummaryInformationStream", () => { it("round-trips every field this module covers", () => { @@ -36,7 +37,7 @@ describe("readSummaryInformation / writeSummaryInformationStream", () => { expect(propertySet.properties.has(12)).toBe(false); // PIDSI_CREATE_DTM }); - it("joins keywords with ', ' and splits them back apart, dropping empty entries", () => { + it("joins keywords with ', ' and splits them back apart", () => { const bytes = writeSummaryInformationStream({ keywords: ["a", "b", "c"] }); const propertySet = readPropertySetStream(bytes); expect(propertySet.properties.get(5)).toEqual({ @@ -46,6 +47,44 @@ describe("readSummaryInformation / writeSummaryInformationStream", () => { expect(readSummaryInformation(bytes).keywords).toEqual(["a", "b", "c"]); }); + it("drops empty entries a hand-written KEYWORDS value carries between commas", () => { + // writeSummaryInformationStream's own join never produces an empty segment, so this builds the KEYWORDS property directly to exercise splitKeywords' own filter -- a real producer's comma-separated field is not guaranteed free of doubled or trailing delimiters. + const bytes = writePropertySetStream({ + formatId: FMTID_SUMMARY_INFORMATION, + properties: new Map([[5, { type: "VT_LPWSTR", value: "a,,b, ,c" }]]), // PIDSI_KEYWORDS + }); + expect(readSummaryInformation(bytes).keywords).toEqual(["a", "b", "c"]); + }); + + it("reads keywords back as absent when every comma-separated entry is empty or whitespace", () => { + const bytes = writePropertySetStream({ + formatId: FMTID_SUMMARY_INFORMATION, + properties: new Map([[5, { type: "VT_LPWSTR", value: " , , " }]]), // PIDSI_KEYWORDS + }); + expect(readSummaryInformation(bytes).keywords).toBeUndefined(); + }); + + it("writes no KEYWORDS property for a defined but empty keywords array", () => { + const bytes = writeSummaryInformationStream({ keywords: [] }); + expect(readPropertySetStream(bytes).properties.has(5)).toBe(false); // PIDSI_KEYWORDS + }); + + it("reads a genuine VT_LPSTR string property exactly as it reads the VT_LPWSTR this module itself writes", () => { + // writeSummaryInformationStream never writes VT_LPSTR (see write.ts's own scope note; writePropertySetStream itself refuses to encode one), so a round trip through this module's own writer never exercises stringValue's VT_LPSTR branch -- built instead through test-support's own encoder, which supports VT_LPSTR directly, against a real producer's more common ANSI string encoding. + const bytes = propertySetStream(FMTID_SUMMARY_INFORMATION, [ + { pid: 4, value: { type: "VT_LPSTR", value: "Ansi Author" } }, // PIDSI_AUTHOR + ]); + expect(readSummaryInformation(bytes).author).toBe("Ansi Author"); + }); + + it("reads an explicit empty string property back as absent, not as an empty string", () => { + const bytes = writePropertySetStream({ + formatId: FMTID_SUMMARY_INFORMATION, + properties: new Map([[3, { type: "VT_LPWSTR", value: "" }]]), // PIDSI_SUBJECT + }); + expect(readSummaryInformation(bytes).subject).toBeUndefined(); + }); + it("declares CP_WINUNICODE as its CodePage property", () => { const bytes = writeSummaryInformationStream({ title: "x" }); expect(readPropertySetStream(bytes).properties.get(1)).toEqual({ @@ -73,14 +112,28 @@ describe("readSummaryInformation / writeSummaryInformationStream", () => { formatId: "{D5CDD502-2E9C-101B-9397-08002B2CF9AE}", // FMTID_DocSummaryInformation properties: new Map([[2, { type: "VT_LPWSTR", value: "x" }]]), }); - expect(() => readSummaryInformation(bytes)).toThrow(PropertySetFormatError); + expect(() => readSummaryInformation(bytes)).toThrow( + 'property set stream declares FMTID {D5CDD502-2E9C-101B-9397-08002B2CF9AE}, not FMTID_SummaryInformation ({F29F85E0-4FF9-1068-AB91-08002B27B3D9}); this is not a "\\x05SummaryInformation" stream', + ); }); - it("throws PropertySetFormatError when a known field's property has the wrong type", () => { + it("throws PropertySetFormatError when a known string field's property has the wrong type", () => { const bytes = writePropertySetStream({ formatId: FMTID_SUMMARY_INFORMATION, properties: new Map([[2, { type: "VT_I4", value: 1 }]]), // PIDSI_TITLE, wrong type }); - expect(() => readSummaryInformation(bytes)).toThrow(PropertySetFormatError); + expect(() => readSummaryInformation(bytes)).toThrow( + "SummaryInformation property 2 has type VT_I4, not a string type as [MS-OLEPS]'s SummaryInformation Property Set defines", + ); + }); + + it("throws PropertySetFormatError when a known date field's property has the wrong type", () => { + const bytes = writePropertySetStream({ + formatId: FMTID_SUMMARY_INFORMATION, + properties: new Map([[12, { type: "VT_I4", value: 1 }]]), // PIDSI_CREATE_DTM, wrong type + }); + expect(() => readSummaryInformation(bytes)).toThrow( + "SummaryInformation property 12 has type VT_I4, not VT_FILETIME as [MS-OLEPS]'s SummaryInformation Property Set defines", + ); }); }); diff --git a/packages/archive-codec/src/oleps/wire.test.ts b/packages/archive-codec/src/oleps/wire.test.ts new file mode 100644 index 0000000000..620fefe5c1 --- /dev/null +++ b/packages/archive-codec/src/oleps/wire.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import { readGuid, writeGuid } from "./wire"; + +// Direct coverage for readGuid/writeGuid (src/oleps/wire.ts), isolated from ./read.ts and ./write.ts: those two only ever call writeGuid immediately before overwriting the very next field (Offset0), so a writer that wrote one byte past its own 16-byte Data4 region would have that overwrite silently erased by the following legitimate write -- never observable through a full writePropertySetStream/readPropertySetStream round trip. Passing writeGuid a DataView sized to exactly the GUID's own 16 bytes makes that off-by-one byte write land outside the view entirely, where DataView's own bounds check throws rather than silently clobbering an unrelated byte. + +describe("writeGuid / readGuid", () => { + it("round-trips a GUID with a distinct byte in every position", () => { + // Every one of Data1/Data2/Data3/Data4's bytes differs from its neighbours, so a slice/offset arithmetic error in either direction moves a real byte into the wrong slot rather than duplicating an already-matching one. + const guid = "{01234567-89AB-CDEF-0123-456789ABCDEF}"; + const bytes = new Uint8Array(16); + const view = new DataView(bytes.buffer); + writeGuid(view, 0, guid); + expect(readGuid(view, 0)).toBe(guid); + }); + + it("writes exactly its own 16 bytes and no further, even into a buffer sized to hold only one GUID", () => { + // A DataView spanning precisely 16 bytes: any write past index 15 throws, catching a Data4 loop bound one iteration too long that a shared, larger property-set buffer would otherwise silently absorb. + const bytes = new Uint8Array(16); + const view = new DataView(bytes.buffer); + expect(() => { + writeGuid(view, 0, "{00000000-0000-0000-0000-000000000000}"); + }).not.toThrow(); + }); + + it("does not disturb bytes immediately before or after the GUID field", () => { + const bytes = new Uint8Array(18).fill(0xaa); + const view = new DataView(bytes.buffer); + writeGuid(view, 1, "{11111111-2222-3333-4444-555555555555}"); + expect(bytes[0]).toBe(0xaa); + expect(bytes[17]).toBe(0xaa); + }); +}); diff --git a/packages/archive-codec/src/oleps/wire.ts b/packages/archive-codec/src/oleps/wire.ts index bb6b047211..fccfd65074 100644 --- a/packages/archive-codec/src/oleps/wire.ts +++ b/packages/archive-codec/src/oleps/wire.ts @@ -51,16 +51,19 @@ export function readGuid(view: DataView, offset: number): string { return `{${data1}-${data2}-${data3}-${data4a}-${data4b}}`.toUpperCase(); } +// One Data4 byte's own two hex characters, starting at charIndex: kept as a single shared expression (rather than writeGuid computing a start and an independently-derived end) so a wrong charIndex always extracts a genuinely different two-character window, never one that merely gains extra leading digits setUint8's own mod-256 truncation would silently discard. +function hexByte(digits: string, charIndex: number): number { + return Number.parseInt(digits.slice(charIndex, charIndex + 2), 16); +} + export function writeGuid(view: DataView, offset: number, guid: string): void { const digits = guid.replace(/[{}-]/g, ""); view.setUint32(offset, Number.parseInt(digits.slice(0, 8), 16), true); view.setUint16(offset + 4, Number.parseInt(digits.slice(8, 12), 16), true); view.setUint16(offset + 6, Number.parseInt(digits.slice(12, 16), 16), true); - for (let i = 0; i < 8; i++) { - view.setUint8( - offset + 8 + i, - Number.parseInt(digits.slice(16 + i * 2, 18 + i * 2), 16), - ); + // Data4's own 8 bytes, over a literal index list rather than a `for` loop's own comparison bound: a bound one iteration too long or short would otherwise land on the byte immediately past the GUID's own 16 bytes, which every real call site overwrites with its own next field regardless, leaving the off-by-one silently unobservable. + for (const i of [0, 1, 2, 3, 4, 5, 6, 7]) { + view.setUint8(offset + 8 + i, hexByte(digits, 16 + i * 2)); } } diff --git a/packages/archive-codec/src/oleps/write.test.ts b/packages/archive-codec/src/oleps/write.test.ts index 9e11d0c6fb..a3e39c6aec 100644 --- a/packages/archive-codec/src/oleps/write.test.ts +++ b/packages/archive-codec/src/oleps/write.test.ts @@ -52,6 +52,53 @@ describe("writePropertySetStream", () => { expect(read.properties.get(12)).toEqual({ type: "VT_I4", value: 3 }); }); + it("pads a VT_LPWSTR value's Characters field out to exactly the next 4-byte boundary, no further", () => { + // A round trip alone cannot catch over-padding: the reader locates every property by its own dictionary offset, not by a fixed layout, so an oversized (but still zero-filled) pad still reads back correctly. This instead measures the real byte gap between two adjacent properties directly off the wire. + const properties = new Map([ + [2, { type: "VT_LPWSTR", value: "ab" }], // characters field: (2 + 1) * 2 = 6 bytes, padded to 8 + [5, { type: "VT_I4", value: 0 }], + ]); + const bytes = writePropertySetStream({ + formatId: FMTID_SUMMARY_INFORMATION, + properties, + }); + const HEADER_SIZE = 48; + const PROPERTY_SET_HEADER_SIZE = 8; + const IDENTIFIER_AND_OFFSET_SIZE = 8; + const TYPED_VALUE_HEADER_SIZE = 4; + const view = new DataView(bytes.buffer); + const dictionaryStart = HEADER_SIZE + PROPERTY_SET_HEADER_SIZE; + const firstOffset = view.getUint32(dictionaryStart + 4, true); + const secondOffset = view.getUint32( + dictionaryStart + IDENTIFIER_AND_OFFSET_SIZE + 4, + true, + ); + // TYPED_VALUE_HEADER_SIZE (Type+Padding) + 4 (the Length field) + 8 (the padded Characters field). + expect(secondOffset - firstOffset).toBe(TYPED_VALUE_HEADER_SIZE + 4 + 8); + }); + + it("writes the PropertyIdentifierAndOffset dictionary itself in ascending PID order on the wire, not merely insertion order", () => { + // readPropertySetStream inserts properties into its own Map in whatever order the dictionary lists them, but a Map's key order is never asserted on above (the previous test re-sorts before comparing) -- this reads the three dictionary entries' own PID fields directly off the wire instead. + const properties = new Map([ + [12, { type: "VT_I4", value: 3 }], + [2, { type: "VT_I4", value: 1 }], + [5, { type: "VT_I4", value: 2 }], + ]); + const bytes = writePropertySetStream({ + formatId: FMTID_SUMMARY_INFORMATION, + properties, + }); + const HEADER_SIZE = 48; + const PROPERTY_SET_HEADER_SIZE = 8; + const IDENTIFIER_AND_OFFSET_SIZE = 8; + const view = new DataView(bytes.buffer); + const dictionaryStart = HEADER_SIZE + PROPERTY_SET_HEADER_SIZE; + const pids = [0, 1, 2].map((i) => + view.getUint32(dictionaryStart + i * IDENTIFIER_AND_OFFSET_SIZE, true), + ); + expect(pids).toEqual([2, 5, 12]); + }); + it("writes a well-formed stream with no properties at all", () => { const bytes = writePropertySetStream({ formatId: FMTID_SUMMARY_INFORMATION, @@ -80,11 +127,19 @@ describe("writePropertySetStream", () => { const properties = new Map([ [2, { type: "VT_LPSTR", value: "ansi" }], ]); - expect(() => + let caught: unknown; + try { writePropertySetStream({ formatId: FMTID_SUMMARY_INFORMATION, properties, - }), - ).toThrow(PropertySetWriteError); + }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(PropertySetWriteError); + expect((caught as Error).name).toBe("PropertySetWriteError"); + expect((caught as Error).message).toBe( + "writePropertySetStream cannot write a VT_LPSTR property: this writer emits Unicode (VT_LPWSTR) strings only, since encoding to an arbitrary ANSI codepage is out of scope -- see the package README's OLEPS scope note", + ); }); }); diff --git a/packages/archive-codec/src/oleps/write.ts b/packages/archive-codec/src/oleps/write.ts index e814857ad0..42077330ac 100644 --- a/packages/archive-codec/src/oleps/write.ts +++ b/packages/archive-codec/src/oleps/write.ts @@ -1,6 +1,5 @@ import { BYTE_ORDER_MARK, - GUID_NULL, HEADER_SIZE, IDENTIFIER_AND_OFFSET_SIZE, PROPERTY_SET_HEADER_SIZE, @@ -28,14 +27,13 @@ export class PropertySetWriteError extends Error { } } -// [MS-OLEPS] 2.20 UnicodeString's own Characters field: a null-terminated array of 16-bit code units. JS strings are already sequences of UTF-16 code units, so this copies charCodeAt directly rather than re-encoding -- a surrogate pair round-trips as its own two code units with no special-casing needed, since nothing here interprets code-point boundaries. +// [MS-OLEPS] 2.20 UnicodeString's own Characters field: a null-terminated array of 16-bit code units. `split("")` walks a JS string by UTF-16 code unit (unlike spreading a string, which walks by Unicode code point and would split a surrogate pair across two array entries) -- a surrogate pair round-trips as its own two code units with no special-casing needed, since nothing here interprets code-point boundaries. No explicit terminator write: characterBytes is allocated one 16-bit unit longer than `value` itself and starts zero-filled, so the reserved terminator slot already holds the 0 [MS-OLEPS] 2.20 requires without writing it a second time. function encodeUnicodeStringValue(value: string): Uint8Array { const characterBytes = new Uint8Array((value.length + 1) * 2); const charView = new DataView(characterBytes.buffer); - for (let i = 0; i < value.length; i++) { - charView.setUint16(i * 2, value.charCodeAt(i), true); - } - charView.setUint16(value.length * 2, 0, true); // the null terminator [MS-OLEPS] 2.20 requires + value.split("").forEach((unit, i) => { + charView.setUint16(i * 2, unit.charCodeAt(0), true); + }); return characterBytes; } @@ -49,39 +47,38 @@ function encodeTypedPropertyValue( ): Uint8Array { switch (value.type) { case "VT_I2": { + // Padding (bytes 2-3) and the trailing alignment padding (bytes 6-7) both stay zero: `bytes` is fresh off `new Uint8Array`, which already zero-fills every byte this case does not itself set. const bytes = new Uint8Array(TYPED_VALUE_HEADER_SIZE + 4); const view = new DataView(bytes.buffer); view.setUint16(0, VT_I2, true); - view.setUint16(2, 0, true); view.setInt16(4, value.value, true); - view.setUint16(6, 0, true); return bytes; } case "VT_I4": { + // Padding (bytes 2-3) stays zero: `bytes` is fresh off `new Uint8Array`, which already zero-fills every byte this case does not itself set. const bytes = new Uint8Array(TYPED_VALUE_HEADER_SIZE + 4); const view = new DataView(bytes.buffer); view.setUint16(0, VT_I4, true); - view.setUint16(2, 0, true); view.setInt32(4, value.value, true); return bytes; } case "VT_FILETIME": { + // Padding (bytes 2-3) stays zero: `bytes` is fresh off `new Uint8Array`, which already zero-fills every byte this case does not itself set. const { low, high } = dateToFiletime(value.value); const bytes = new Uint8Array(TYPED_VALUE_HEADER_SIZE + 8); const view = new DataView(bytes.buffer); view.setUint16(0, VT_FILETIME, true); - view.setUint16(2, 0, true); view.setUint32(4, low, true); view.setUint32(8, high, true); return bytes; } case "VT_LPWSTR": { + // Padding (bytes 2-3) stays zero: `bytes` is fresh off `new Uint8Array`, which already zero-fills every byte this case does not itself set. const characters = encodeUnicodeStringValue(value.value); const paddedLength = padTo4(characters.length); const bytes = new Uint8Array(TYPED_VALUE_HEADER_SIZE + 4 + paddedLength); const view = new DataView(bytes.buffer); view.setUint16(0, VT_LPWSTR, true); - view.setUint16(2, 0, true); view.setUint32(4, characters.length / 2, true); // Length is in 16-bit units, not bytes bytes.set(characters, TYPED_VALUE_HEADER_SIZE + 4); return bytes; @@ -136,9 +133,7 @@ export function writePropertySetStream( const streamBytes = new Uint8Array(HEADER_SIZE + propertySetBytes.length); const view = new DataView(streamBytes.buffer); view.setUint16(0, BYTE_ORDER_MARK, true); - view.setUint16(2, 0, true); // Version 0: none of the types this writer emits need version 1's extra features - view.setUint32(4, 0, true); // SystemIdentifier is implementation-specific and MUST be ignored by readers ([MS-OLEPS] 2.21); zero rather than impersonating a real OS identifier - writeGuid(view, 8, GUID_NULL); // CLSID: this package has no notion of a property set's own associated CLSID to record + // Version (bytes 2-3, 0: none of the types this writer emits need version 1's extra features), SystemIdentifier (bytes 4-7, implementation-specific and MUST be ignored by readers per [MS-OLEPS] 2.21, so left at 0 rather than impersonating a real OS identifier), and CLSID (bytes 8-23, this package has no notion of a property set's own associated CLSID, and GUID_NULL is all zero bytes) all stay zero: streamBytes is fresh off `new Uint8Array`, which already zero-fills every byte none of these three fields is written a second time. view.setUint32(24, 1, true); // NumPropertySets writeGuid(view, 28, propertySet.formatId); view.setUint32(44, HEADER_SIZE, true); // Offset0 diff --git a/packages/archive-codec/src/test-support/cfb.test.ts b/packages/archive-codec/src/test-support/cfb.test.ts new file mode 100644 index 0000000000..b4b151ddb5 --- /dev/null +++ b/packages/archive-codec/src/test-support/cfb.test.ts @@ -0,0 +1,314 @@ +import { describe, expect, it } from "vitest"; +import { readCompoundFile } from "../cfb/read"; +import { compoundFile } from "./cfb"; + +// Direct coverage for the [MS-CFB] fixture builder itself (src/test-support/cfb.ts), independent of the ../cfb/read.test.ts and ../cfb/write.test.ts suites that consume it as a black box. Most of this builder's own logic is already exercised indirectly by those two suites reading back what it writes -- these cases target the specific internal decisions (sibling-node reuse, byte-exact header/directory-entry fields, loop boundaries) that a correct read-back alone cannot distinguish from a subtly wrong one. + +const enc = (s: string): Uint8Array => new TextEncoder().encode(s); + +// Follows the directory's own FAT chain to its ENDOFCHAIN terminator, the same way read.ts would, rather than guessing a sector count from the total file length -- a sector that happens to hold mini-stream or mini-FAT content, not real directory rows, can otherwise be miscounted as one more directory sector by coincidence of its own leading byte. +function directorySectorCount(bytes: Uint8Array): number { + const view = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength); + const directoryStart = view.getUint32(0x30, true); + // FAT sector k always sits at physical sector k itself (this builder's own identity mapping, per its module comment), so FAT sector k's own bytes are at file offset (k + 1) * 512, and entry `sector`'s own slot is 4 * (sector mod 128) bytes into whichever FAT sector holds it. + const fatEntry = (sector: number): number => + view.getUint32( + (Math.floor(sector / 128) + 1) * 512 + (sector % 128) * 4, + true, + ); + let count = 1; + let current = directoryStart; + for (;;) { + const next = fatEntry(current); + if (next === 0xfffffffe) { + break; // ENDOFCHAIN + } + current = next; + count += 1; + } + return count; +} + +function directoryEntryCount(bytes: Uint8Array): number { + return (directorySectorCount(bytes) * 512) / 128; +} + +// Reads back every directory entry's own name and object type directly from the bytes, in id order -- a lower-level probe than readCompoundFile, which only ever surfaces stream paths, never a storage's own presence or a duplicate name. +function directoryEntries( + bytes: Uint8Array, +): { name: string; objectType: number }[] { + const view = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength); + const directoryStart = view.getUint32(0x30, true); + const count = directoryEntryCount(bytes); + const decoder = new TextDecoder("utf-16le"); + const entries: { name: string; objectType: number }[] = []; + for (let id = 0; id < count; id++) { + const base = (directoryStart + 1) * 512 + id * 128; + const objectType = view.getUint8(base + 0x42); + if (objectType === 0) { + continue; // unallocated padding + } + const nameLength = view.getUint16(base + 0x40, true); + const name = decoder.decode( + bytes.subarray(base, base + Math.max(0, nameLength - 2)), + ); + entries.push({ name, objectType }); + } + return entries; +} + +describe("compoundFile sibling reuse", () => { + it("creates exactly one storage entry for a name shared by several stream paths, not one per path", () => { + const bytes = compoundFile([ + { path: "Pool/First", bytes: enc("1") }, + { path: "Pool/Second", bytes: enc("2") }, + { path: "Pool/Third", bytes: enc("3") }, + ]); + const poolEntries = directoryEntries(bytes).filter( + (e) => e.name === "Pool", + ); + expect(poolEntries).toHaveLength(1); + expect(poolEntries[0]?.objectType).toBe(1); // storage + }); + + it("creates a separate storage entry for each distinctly-named top-level path, not a single shared one", () => { + const bytes = compoundFile([ + { path: "Alpha/X", bytes: enc("1") }, + { path: "Beta/Y", bytes: enc("2") }, + ]); + const entries = directoryEntries(bytes); + expect(entries.filter((e) => e.name === "Alpha")).toHaveLength(1); + expect(entries.filter((e) => e.name === "Beta")).toHaveLength(1); + }); + + it("never reuses a stream node as a storage, even when a later path needs the same name to be one", () => { + // "Thing" is written first as a stream; "Thing/Inner" then needs an intermediate storage of the same name. The sibling-reuse search must skip the existing stream node (it is not a storage) and create a genuinely new storage entry instead, rather than silently reusing the wrong kind of node. + const bytes = compoundFile([ + { path: "Thing", bytes: enc("leaf") }, + { path: "Thing/Inner", bytes: enc("nested") }, + ]); + const thingEntries = directoryEntries(bytes).filter( + (e) => e.name === "Thing", + ); + expect(thingEntries).toHaveLength(2); + expect(thingEntries.map((e) => e.objectType).sort()).toEqual([1, 2]); // one storage, one stream + const streams = readCompoundFile(bytes); + expect(streams.map((s) => s.path).sort()).toEqual(["Thing", "Thing/Inner"]); + }); +}); + +describe("compoundFile sibling right-links", () => { + it("chains three siblings to each other, not merely each to the first", () => { + const bytes = compoundFile([ + { path: "Pool/First", bytes: enc("1") }, + { path: "Pool/Second", bytes: enc("2") }, + { path: "Pool/Third", bytes: enc("3") }, + ]); + const streams = readCompoundFile(bytes); + expect(streams.map((s) => s.path).sort()).toEqual([ + "Pool/First", + "Pool/Second", + "Pool/Third", + ]); + }); + + it("gives the last sibling in a chain NOSTREAM as its own right link, not a link to itself or the first", () => { + const bytes = compoundFile([ + { path: "A", bytes: enc("1") }, + { path: "B", bytes: enc("2") }, + ]); + const view = new DataView(bytes.buffer); + const directoryStart = view.getUint32(0x30, true); + // Entry 0 is root, entry 1 is A, entry 2 is B (insertion order, depth-first). + const bRightLink = view.getUint32( + (directoryStart + 1) * 512 + 2 * 128 + 0x48, + true, + ); + expect(bRightLink).toBe(0xffffffff); // NOSTREAM + }); +}); + +describe("compoundFile directory-entry byte layout", () => { + it("names the root directory entry 'Root Entry', not a placeholder", () => { + // Nothing functional depends on this name (../cfb/read.ts's own comment says so explicitly, and no test elsewhere reads it), so only a direct byte-level decode of entry 0's own name field can tell a real name from an empty placeholder. + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + const directoryStart = view.getUint32(0x30, true); + const base = (directoryStart + 1) * 512; // entry 0, the root, at the directory's own start + const nameLength = view.getUint16(base + 0x40, true); + const name = new TextDecoder("utf-16le").decode( + bytes.subarray(base, base + nameLength - 2), + ); + expect(name).toBe("Root Entry"); + }); + + it("writes the colour flag byte as 1 (black) for every entry", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + const directoryStart = view.getUint32(0x30, true); + // Root is id 0, A is id 1. + expect(view.getUint8((directoryStart + 1) * 512 + 0 * 128 + 0x43)).toBe(1); + expect(view.getUint8((directoryStart + 1) * 512 + 1 * 128 + 0x43)).toBe(1); + }); + + it("writes the header's own minor version field as 0x003E", () => { + // [MS-CFB] 2.2 names this value for both major version 3 and 4, but real readers (including ../cfb/read.ts) never inspect it -- direct byte inspection is the only way to notice it going unwritten. + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + expect(new DataView(bytes.buffer).getUint16(0x18, true)).toBe(0x3e); + }); + + it("accepts the highest ASCII byte value (0x7F) in a name, and rejects the lowest non-ASCII one (0x80)", () => { + // 0x7F (DEL) is the last code point checkedName's own > 0x7f test allows; a name built from it must round-trip. 0x80 is the first byte that test rejects. + const highAscii = String.fromCharCode(0x7f); + expect(() => + compoundFile([{ path: highAscii, bytes: enc("x") }]), + ).not.toThrow(); + const nonAscii = String.fromCharCode(0x80); + expect(() => compoundFile([{ path: nonAscii, bytes: enc("x") }])).toThrow( + /non-empty ASCII/, + ); + }); +}); + +describe("compoundFile entry-path validation", () => { + it("rejects a completely empty path, not just an empty segment within one", () => { + expect(() => compoundFile([{ path: "", bytes: enc("x") }])).toThrow( + /no empty segments/, + ); + }); +}); + +describe("compoundFile FAT chain lengths", () => { + it("chains a stream needing exactly two sectors as two links, not one or three", () => { + // 512-byte sectors; a 600-byte stream needs ceil(600/512) = 2 sectors. The chain() helper's own loop must mark exactly that many FAT entries (the last ENDOFCHAIN, every other pointing to the next), not one short (truncating real content) or one long (chaining into whatever sector follows). + const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(600)) }]); + const streams = readCompoundFile(bytes); + expect(streams[0]?.bytes.length).toBe(600); + }); + + it("chains a mini-resident stream needing exactly two mini sectors as two links, not one or three", () => { + // 64-byte mini sectors; a 100-byte stream needs ceil(100/64) = 2 mini sectors. + const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(100)) }]); + const streams = readCompoundFile(bytes); + expect(streams[0]?.bytes.length).toBe(100); + }); +}); + +describe("compoundFile FAT and mini-FAT padding tails", () => { + it("fills the FAT's own unused tail entries with FREESECT, past the file's real total sector count", () => { + // Every chain() call but the very last is immediately followed by the next region's own chain() call, whose first write lands exactly where an off-by-one in the previous call would have -- overwriting it regardless. The very last call (the mini-FAT's own chain) has nothing after it, so an off-by-one there leaks into the FAT's own genuinely unused padding tail, which must still read FREESECT. + const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); + const view = new DataView(bytes.buffer); + const fatSectorCount = view.getUint32(0x2c, true); + const entriesPerFatSector = 512 / 4; + const totalRealSectors = bytes.length / 512 - 1; + const totalAddressableSectors = fatSectorCount * entriesPerFatSector; + expect(totalAddressableSectors).toBeGreaterThan(totalRealSectors); + for ( + let sector = totalRealSectors; + sector < totalAddressableSectors; + sector++ + ) { + const holder = Math.floor(sector / entriesPerFatSector); + expect( + view.getUint32( + (holder + 1) * 512 + (sector % entriesPerFatSector) * 4, + true, + ), + ).toBe(0xffffffff); // FREESECT + } + }); + + it("marks every one of its own FAT sectors as FATSECT in the FAT table itself", () => { + // Removing the fat[sector] = FATSECT loop entirely would leave every FAT sector reading back as FREESECT (its own initial fill value), since nothing else in this builder ever writes to those specific indices. + const bytes = compoundFile([{ path: "A", bytes: enc("x".repeat(5000)) }]); + const view = new DataView(bytes.buffer); + const fatSectorCount = view.getUint32(0x2c, true); + const entriesPerFatSector = 512 / 4; + for (let sector = 0; sector < fatSectorCount; sector++) { + const holder = Math.floor(sector / entriesPerFatSector); + expect( + view.getUint32( + (holder + 1) * 512 + (sector % entriesPerFatSector) * 4, + true, + ), + ).toBe(0xfffffffd); // FATSECT + } + }); + + it("fills the mini-FAT's own unused tail entries with FREESECT, past the real mini sector count", () => { + // The mini-FAT chain loop is the very last thing this builder writes into the miniFat array -- nothing follows it to overwrite an off-by-one, so its own padding tail is the direct witness. + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); // 1 byte -> 1 real mini sector, needing 1 mini-FAT sector of mostly padding + const view = new DataView(bytes.buffer); + const miniFatStart = view.getUint32(0x3c, true); + const miniFatSectorCount = view.getUint32(0x40, true); + const entriesPerFatSector = 512 / 4; + const realMiniSectors = 1; + const totalAddressableMiniSlots = miniFatSectorCount * entriesPerFatSector; + expect(totalAddressableMiniSlots).toBeGreaterThan(realMiniSectors); + for (let slot = realMiniSectors; slot < totalAddressableMiniSlots; slot++) { + const holder = miniFatStart + Math.floor(slot / entriesPerFatSector); + expect( + view.getUint32( + (holder + 1) * 512 + (slot % entriesPerFatSector) * 4, + true, + ), + ).toBe(0xffffffff); // FREESECT + } + }); +}); + +describe("compoundFile header DIFAT array padding", () => { + it("fills every one of the header's 109 DIFAT entries, the real FAT sector indices then FREESECT", () => { + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + const view = new DataView(bytes.buffer); + const fatSectorCount = view.getUint32(0x2c, true); + expect(fatSectorCount).toBeGreaterThan(0); + for (let i = 0; i < fatSectorCount; i++) { + expect(view.getUint32(0x4c + i * 4, true)).toBe(i); + } + for (let i = fatSectorCount; i < 109; i++) { + expect(view.getUint32(0x4c + i * 4, true)).toBe(0xffffffff); + } + }); +}); + +describe("compoundFile stream size partitioning", () => { + it("allocates no FAT-resident data sector at all for a file holding only one mini-resident stream", () => { + // A stream this small being also (wrongly) counted among the "big" (FAT-resident) partition would allocate an extra, entirely unused data sector for it -- unobservable through read-back (the entry's own startSector still correctly points into the mini stream), but it inflates the file's own total size. 1 FAT sector + 1 directory sector + 1 mini-stream sector + 1 mini-FAT sector is the true minimum for this fixture. + const bytes = compoundFile([{ path: "A", bytes: enc("x") }]); + expect(bytes.length).toBe(512 * (1 + 1 + 1 + 1 + 1)); // header + 4 sectors + }); +}); + +describe("compoundFile mini-FAT sector boundary", () => { + it("keeps a single stream's chain intact even when it straddles the 128-entry mini-FAT-sector boundary", () => { + // 126 one-mini-sector filler streams occupy mini sectors 0-125; a 4-mini-sector stream right after them occupies 126-129, so its own chain entries at local indices 126, 127 sit in the first mini-FAT sector and 128, 129 in the second. Every filler stream's own chain entry is ENDOFCHAIN, so a bug that copies the wrong 512-byte chunk into the second mini-FAT sector (duplicating the first, or reading from the wrong offset within the combined buffer) would still read back as ENDOFCHAIN there too if this stream's own real values did not differ from mini sector to mini sector -- filling each of its own four mini sectors with a distinct byte value makes that corruption visible as wrong (truncated or shuffled) content instead. + const fillerCount = 126; + const filler = Array.from({ length: fillerCount }, (_unused, i) => ({ + path: `Filler${i}`, + bytes: new Uint8Array(64).fill(1), + })); + const bigBytes = new Uint8Array(64 * 4); + for (let miniSector = 0; miniSector < 4; miniSector++) { + bigBytes.fill(miniSector + 10, miniSector * 64, (miniSector + 1) * 64); + } + const bytes = compoundFile([...filler, { path: "Big", bytes: bigBytes }]); + expect(new DataView(bytes.buffer).getUint32(0x40, true)).toBe(2); // sanity: needs two mini-FAT sectors + const streams = readCompoundFile(bytes); + expect(streams.find((s) => s.path === "Big")?.bytes).toEqual(bigBytes); + }); +}); + +describe("compoundFile directory sector count", () => { + it("needs exactly two directory sectors for a fixture with more than four real entries", () => { + // 4 entries per 512-byte directory sector; root plus 5 streams is 6 real entries, needing 2 sectors -- the directorySectorCount loop's own boundary matters here, not merely whether one sector is enough. + const bytes = compoundFile( + Array.from({ length: 5 }, (_unused, i) => ({ + path: `S${i}`, + bytes: enc("x"), + })), + ); + expect(directoryEntryCount(bytes)).toBe(8); // 2 sectors * 4 entries + }); +}); diff --git a/packages/archive-codec/src/test-support/cfb.ts b/packages/archive-codec/src/test-support/cfb.ts index 363e3d01fa..e70bbc4f6c 100644 --- a/packages/archive-codec/src/test-support/cfb.ts +++ b/packages/archive-codec/src/test-support/cfb.ts @@ -29,6 +29,7 @@ interface DirectoryRecord { readonly node: StorageNode; readonly id: number; rightId: number; + childId: number; } /** A directory record whose node genuinely carries a stream, so reads of it need no absent case. */ @@ -57,13 +58,10 @@ function put32(view: DataView, offset: number, value: number): void { view.setUint32(offset, value, true); } +// No node.name.length === 0 guard: this is called only via writeDirectoryEntry, once for the root (whose own name is "Root Entry" from creation, never empty) and once each for every other node, whose name was already proven non-empty by compoundFile's own path-segment validation before the node was ever created. An empty name can never reach here. function checkedName(node: StorageNode): Uint8Array { const encoded = enc(node.name); - if ( - node.name.length === 0 || - encoded.length > 31 || - encoded.some((byte) => byte > 0x7f) - ) { + if (encoded.length > 31 || encoded.some((byte) => byte > 0x7f)) { throw new Error( `compoundFile stream/storage names must be non-empty ASCII of at most 31 characters (got ${JSON.stringify(node.name)})`, ); @@ -81,10 +79,11 @@ function writeDirectoryEntry( size: number, ): void { const encoded = checkedName(node); - for (let i = 0; i < encoded.length; i++) { - entry.setUint8(i * 2, encoded[i] ?? 0); + // Walks the encoded bytes directly (each paired with its own index via Array.from, rather than a hand-written comparison bound indexing back into encoded with a `?? 0` fallback for an out-of-range read that can now never happen): every element visited this way is a real byte of encoded, never the array's own out-of-range undefined. + Array.from(encoded).forEach((byte, i) => { + entry.setUint8(i * 2, byte); entry.setUint8(i * 2 + 1, 0); - } + }); // The name field's bytes past the name stay zero: that zero pair IS the terminating null EntryNameLength counts. put16(entry, 0x40, encoded.length * 2 + 2); entry.setUint8(0x42, objectType); @@ -94,7 +93,7 @@ function writeDirectoryEntry( put32(entry, 0x4c, childId); put32(entry, 0x74, startSector); put32(entry, 0x78, size); - put32(entry, 0x7c, 0); + // No write of the high 32 bits at 0x7c: every size this test-support builder ever writes (a real stream's own byte length, or the mini stream's total) fits comfortably under 2^32, so that word is always 0 -- already true from entry's own allocation, and this builder's own streams never need anything else. } function padToMultiple( @@ -118,15 +117,13 @@ export function compoundFile( const entriesPerDirectorySector = sectorSize / 128; const fatEntriesPerSector = sectorSize / 4; - const root: StorageNode = { name: "", children: [] }; + // "Root Entry" from the start, not a placeholder overridden at write time: nothing else ever reads this node's own name (it is never a sibling, so it never enters a name comparison), so there is no reason to carry a second, different string that would only be discarded later. + const root: StorageNode = { name: "Root Entry", children: [] }; for (const entry of entries) { const segments = entry.path.split("/"); const leaf = segments.pop(); - if ( - leaf === undefined || - leaf.length === 0 || - segments.some((segment) => segment.length === 0) - ) { + // !leaf alone (not leaf === undefined || leaf.length === 0) covers exactly the same two cases: String.prototype.split always returns at least one element, so .pop() on it is genuinely never undefined here -- only ever a string, possibly empty -- and a falsy check catches both undefined and "" identically to spelling them out, while also narrowing leaf to string below. + if (!leaf || segments.some((segment) => segment.length === 0)) { throw new Error( `compoundFile entry paths must be slash-separated with no empty segments (got ${JSON.stringify(entry.path)})`, ); @@ -153,32 +150,23 @@ export function compoundFile( // Directory entry IDs: the root is 0, then depth-first in insertion order. const records: DirectoryRecord[] = []; - const recordOf = new Map(); const record = (node: StorageNode): DirectoryRecord => { const created: DirectoryRecord = { node, id: records.length, rightId: NOSTREAM, + childId: NOSTREAM, }; records.push(created); - recordOf.set(node, created); - for (const child of node.children) { - record(child); - } + // Sibling chains, linked directly off this recursive call's own return values rather than a later Map lookup: node.children.map(record) always returns one real DirectoryRecord per child (record() never returns anything else), so iterating it directly never meets its own out-of-range undefined -- only childRecords[i + 1], at the true last sibling, ever is, and that is the genuine "no next sibling" case NOSTREAM already means. created's own child link is set the same way, directly from childRecords[0], rather than left for a later pass to re-derive by looking node.children[0] up in a separate node -> record map that could only ever find what this same call already has in hand. + const childRecords = node.children.map(record); + childRecords.forEach((childRecord, i) => { + childRecord.rightId = childRecords[i + 1]?.id ?? NOSTREAM; + }); + created.childId = childRecords[0]?.id ?? NOSTREAM; return created; }; record(root); - // Sibling chains: each storage's children link right, one to the next. - for (const { node } of records) { - for (let i = 0; i < node.children.length; i++) { - const childRecord = recordOf.get(node.children[i] ?? node); - const next = node.children[i + 1]; - if (childRecord !== undefined) { - childRecord.rightId = - next === undefined ? NOSTREAM : (recordOf.get(next)?.id ?? NOSTREAM); - } - } - } // Narrowed through a type predicate rather than a boolean one, because `filter` with a boolean callback leaves the element type alone: the two partitions below would still carry `stream?: Uint8Array` even though the predicate is exactly what rules the absent case out, and every later read would need a fallback that can never be taken. const smallStreamRecords = records @@ -188,38 +176,36 @@ export function compoundFile( .filter(hasStream) .filter(({ node }) => node.stream.length >= MINI_STREAM_CUTOFF); - // The mini stream: every small stream padded to whole mini sectors, concatenated; each stream's start is its first mini sector's index. - const miniChunks = smallStreamRecords.map(({ node }) => - padToMultiple(node.stream, MINI_SECTOR_SIZE), - ); + // The mini stream: every small stream padded to whole mini sectors, concatenated; each stream's start is its first mini sector's index. Paired directly (record alongside its own already-computed chunk) rather than two same-length arrays indexed in lockstep by a shared counter: neither element can ever be the array's own out-of-range undefined, so there is no fallback left to silently paper over an off-by-one. + const miniEntries = smallStreamRecords.map((record) => ({ + record, + chunk: padToMultiple(record.node.stream, MINI_SECTOR_SIZE), + })); const miniStream = new Uint8Array( - miniChunks.reduce((total, chunk) => total + chunk.length, 0), + miniEntries.reduce((total, { chunk }) => total + chunk.length, 0), ); let miniOffset = 0; const miniStartOf = new Map(); - for (let i = 0; i < smallStreamRecords.length; i++) { - miniStartOf.set( - smallStreamRecords[i]?.id ?? -1, - miniOffset / MINI_SECTOR_SIZE, - ); - miniStream.set(miniChunks[i] ?? new Uint8Array(0), miniOffset); - miniOffset += miniChunks[i]?.length ?? 0; + for (const { record, chunk } of miniEntries) { + miniStartOf.set(record.id, miniOffset / MINI_SECTOR_SIZE); + miniStream.set(chunk, miniOffset); + miniOffset += chunk.length; } const miniSectorCount = miniStream.length / MINI_SECTOR_SIZE; - const bigSectorCounts = bigStreamRecords.map(({ node }) => - Math.ceil(node.stream.length / sectorSize), - ); + // Paired the same way as miniEntries above, and for the same reason. + const bigEntries = bigStreamRecords.map((record) => ({ + record, + sectorCount: Math.ceil(record.node.stream.length / sectorSize), + })); const directorySectorCount = Math.ceil( records.length / entriesPerDirectorySector, ); const miniStreamSectorCount = Math.ceil(miniStream.length / sectorSize); - const miniFatSectorCount = - miniSectorCount === 0 - ? 0 - : Math.ceil(miniSectorCount / fatEntriesPerSector); - const dataSectorCount = bigSectorCounts.reduce( - (total, count) => total + count, + // No miniSectorCount === 0 guard: Math.ceil(0 / fatEntriesPerSector) is already 0, byte-identical to the explicit zero case this ternary special-cased. + const miniFatSectorCount = Math.ceil(miniSectorCount / fatEntriesPerSector); + const dataSectorCount = bigEntries.reduce( + (total, { sectorCount }) => total + sectorCount, 0, ); // FAT-sector fixed point: the FAT sectors must between them map every sector of the file, themselves included. @@ -247,9 +233,9 @@ export function compoundFile( const directoryStart = fatSectorCount; let nextSector = directoryStart + directorySectorCount; const bigStartOf = new Map(); - for (let i = 0; i < bigStreamRecords.length; i++) { - bigStartOf.set(bigStreamRecords[i]?.id ?? -1, nextSector); - nextSector += bigSectorCounts[i] ?? 0; + for (const { record, sectorCount } of bigEntries) { + bigStartOf.set(record.id, nextSector); + nextSector += sectorCount; } const miniStreamStart = nextSector; nextSector += miniStreamSectorCount; @@ -267,11 +253,8 @@ export function compoundFile( fat[sector] = FATSECT; } chain(directoryStart, directorySectorCount); - for (let i = 0; i < bigStreamRecords.length; i++) { - chain( - bigStartOf.get(bigStreamRecords[i]?.id ?? -1) ?? 0, - bigSectorCounts[i] ?? 0, - ); + for (const { record, sectorCount } of bigEntries) { + chain(bigStartOf.get(record.id) ?? 0, sectorCount); } chain(miniStreamStart, miniStreamSectorCount); chain(miniFatStart, miniFatSectorCount); @@ -290,17 +273,13 @@ export function compoundFile( // Directory sectors: entry n sits at byte n * 128 of the concatenated chain. const directory = new Uint8Array(directorySectorCount * sectorSize); - for (const { node, id, rightId } of records) { + for (const { node, id, rightId, childId } of records) { const entry = new DataView(directory.buffer, id * 128, 128); - const childId = - node.children.length === 0 - ? NOSTREAM - : (recordOf.get(node.children[0] ?? node)?.id ?? NOSTREAM); if (node === root) { const start = miniStream.length === 0 ? ENDOFCHAIN : miniStreamStart; writeDirectoryEntry( entry, - { ...node, name: "Root Entry" }, + node, 5, childId, NOSTREAM, @@ -328,10 +307,11 @@ export function compoundFile( // The header: little-endian, the version's own sector shifts, DIFAT in the header array only. The directory-sector count is 0 for version 3 (the spec fixes it there) and the real count for version 4; the reader deliberately does not cross-check either way, but the writer stays spec-conformant. const file = new Uint8Array(sectorSize + totalSectors * sectorSize); const view = new DataView(file.buffer); + // A loop bound one iteration too long would write byte 8 -- the header CLSID field's own first byte, always zero and never otherwise written -- which is already zero from the allocation, an equivalent mutant no test could observe. Walking magic.map/forEach directly removes the comparison bound entirely rather than leaving it to be silently absorbed. const magic = [0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1]; - for (let i = 0; i < magic.length; i++) { - file[i] = magic[i] ?? 0; - } + magic.forEach((byte, i) => { + file[i] = byte; + }); put16(view, 0x18, 0x3e); // minor version: the value producers commonly write; readers ignore it put16(view, 0x1a, majorVersion); put16(view, 0x1c, 0xfffe); // byte order: little-endian @@ -344,25 +324,25 @@ export function compoundFile( put32(view, 0x3c, miniSectorCount === 0 ? ENDOFCHAIN : miniFatStart); put32(view, 0x40, miniFatSectorCount); put32(view, 0x44, ENDOFCHAIN); // first DIFAT sector: none, the DIFAT fits the header array - put32(view, 0x48, 0); - for (let i = 0; i < 109; i++) { - put32( - view, - 0x4c + i * 4, - i < fatSectors.length ? (fatSectors[i] ?? FREESECT) : FREESECT, - ); + // NumberOfDIFATSectors (0x48) stays zero: this generator never spills the DIFAT into its own sectors, and the byte is already zero from the allocation. The header's own 109-entry DIFAT array, over a literal-length array rather than a `for` loop's own comparison bound: a bound one iteration too long would write the byte range the first FAT sector's own data occupies, immediately overwritten by the real copySector call below regardless -- an equivalent mutant no test could observe. + // + // No i < fatSectors.length guard, either: fatSectors[i] is already undefined for every i at or past its own length, and `?? FREESECT` already turns that into the same FREESECT padding the guard's own false branch spelled out -- a second, redundant way of saying the identical thing. + for (const i of Array.from({ length: 109 }, (_unused, index) => index)) { + put32(view, 0x4c + i * 4, fatSectors[i] ?? FREESECT); } const copySector = (sector: number, bytes: Uint8Array): void => { file.set(bytes, sectorSize + sector * sectorSize); }; - for (let i = 0; i < fatSectorCount; i++) { - copySector( - fatSectors[i] ?? 0, - new Uint8Array(fat.buffer, i * sectorSize, sectorSize), - ); - } - for (let i = 0; i < directorySectorCount; i++) { + // fatSectors is the identity array [0, 1, ..., fatSectorCount - 1] (built that way above), so its own element at index i is always i itself -- iterating it directly, rather than re-deriving each element from its own index with a fallback for the array's provably unreachable out-of-range case. + fatSectors.forEach((sector, i) => { + copySector(sector, new Uint8Array(fat.buffer, i * sectorSize, sectorSize)); + }); + // Walks Array.from's own bounded index list rather than a hand-written comparison: directory is allocated at exactly directorySectorCount * sectorSize bytes, so an off-by-one here would subarray a range starting at the array's own length -- already empty, and Uint8Array.prototype.set with an empty source is already a no-op regardless of the target offset (see the mini-stream copy's own comment below for the identical reasoning), so there is nothing here for the extra iteration to actually change. + for (const i of Array.from( + { length: directorySectorCount }, + (_unused, n) => n, + )) { copySector( directoryStart + i, directory.subarray(i * sectorSize, (i + 1) * sectorSize), @@ -374,9 +354,8 @@ export function compoundFile( padToMultiple(record.node.stream, sectorSize), ); } - if (miniStream.length > 0) { - copySector(miniStreamStart, miniStream); - } + // No length guard: Uint8Array.prototype.set with a zero-length source is already a no-op regardless of the target offset, so copying an empty mini stream unconditionally is byte-identical to skipping it. + copySector(miniStreamStart, miniStream); for (let i = 0; i < miniFatSectorCount; i++) { copySector( miniFatStart + i, diff --git a/packages/archive-codec/src/test-support/oleps.test.ts b/packages/archive-codec/src/test-support/oleps.test.ts new file mode 100644 index 0000000000..f05dd87130 --- /dev/null +++ b/packages/archive-codec/src/test-support/oleps.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "vitest"; +import { readPropertySetStream } from "../oleps/read"; +import { propertySetStream } from "./oleps"; + +// Direct coverage for propertySetStream (src/test-support/oleps.ts) itself, beyond what the reader's own test suite exercises in passing: its FMTID/CLSID encoding, its VT_I4 and VT_FILETIME field builders (both under-exercised elsewhere -- every other reader test either corrupts them afterward or never checks their decoded value at all), and its own Characters-field padding. + +describe("propertySetStream", () => { + it("round-trips a FMTID with a distinct byte in every position", () => { + // Every one of Data1/Data2/Data3/Data4's bytes differs from its neighbours, so a slice/offset arithmetic error in the GUID encoder moves a real byte into the wrong slot rather than duplicating an already-matching one. + const formatId = "{01234567-89AB-CDEF-0123-456789ABCDEF}"; + const bytes = propertySetStream(formatId, []); + expect(readPropertySetStream(bytes).formatId).toBe(formatId); + }); + + it("round-trips a VT_I4 field's own value, not merely its type after corruption", () => { + const bytes = propertySetStream("{F29F85E0-4FF9-1068-AB91-08002B27B3D9}", [ + { pid: 20, value: { type: "VT_I4", value: -12345 } }, + ]); + expect(readPropertySetStream(bytes).properties.get(20)).toEqual({ + type: "VT_I4", + value: -12345, + }); + }); + + it("round-trips a VT_FILETIME field's low and high 32-bit halves, both non-zero", () => { + // Both halves deliberately non-zero and distinct from each other: a byte-order or field-offset mistake in either one shows up as a wrong date rather than coincidentally reproducing the correct one. + const bytes = propertySetStream("{F29F85E0-4FF9-1068-AB91-08002B27B3D9}", [ + { + pid: 21, + value: { type: "VT_FILETIME", low: 0xa1d01600, high: 0x01c68e4e }, + }, + ]); + const property = readPropertySetStream(bytes).properties.get(21); + expect(property?.type).toBe("VT_FILETIME"); + // The exact FILETIME -> Date epoch conversion is oleps/wire.ts's own concern (covered there); this only needs to prove the raw low/high 32-bit halves this builder wrote survive the round trip undamaged. + expect((property?.value as Date).toISOString()).toBe( + "2006-06-12T18:33:00.000Z", + ); + }); + + it("pads a VT_LPWSTR Characters field out to exactly the next 4-byte boundary, no further", () => { + // A round trip alone tolerates over-padding (the reader locates the next property by dictionary offset, not fixed layout), so this measures the real byte gap between two adjacent properties directly. + const bytes = propertySetStream("{F29F85E0-4FF9-1068-AB91-08002B27B3D9}", [ + { pid: 2, value: { type: "VT_LPWSTR", value: "ab" } }, // Characters: (2 + 1) * 2 = 6 bytes, padded to 8 + { pid: 5, value: { type: "VT_I4", value: 0 } }, + ]); + const HEADER_SIZE = 48; + const view = new DataView(bytes.buffer); + const dictionaryStart = HEADER_SIZE + 8; + const firstOffset = view.getUint32(dictionaryStart + 4, true); + const secondOffset = view.getUint32(dictionaryStart + 8 + 4, true); + // Type+Padding(4) + Size(4) + the padded 8-byte Characters field. + expect(secondOffset - firstOffset).toBe(4 + 4 + 8); + }); +}); diff --git a/packages/archive-codec/src/test-support/oleps.ts b/packages/archive-codec/src/test-support/oleps.ts index d1517d3d21..4d06b4a90f 100644 --- a/packages/archive-codec/src/test-support/oleps.ts +++ b/packages/archive-codec/src/test-support/oleps.ts @@ -29,27 +29,31 @@ function padTo4(length: number): number { return Math.ceil(length / 4) * 4; } +// One Data4 byte's own two hex characters, starting at charIndex: kept as a single shared expression (rather than writeGuid computing a start and an independently-derived end) so a wrong charIndex always extracts a genuinely different two-character window, never one that merely gains extra leading digits setUint8's own mod-256 truncation would silently discard. +function hexByte(digits: string, charIndex: number): number { + return Number.parseInt(digits.slice(charIndex, charIndex + 2), 16); +} + function writeGuid(view: DataView, offset: number, guid: string): void { const digits = guid.replace(/[{}-]/g, ""); view.setUint32(offset, Number.parseInt(digits.slice(0, 8), 16), true); view.setUint16(offset + 4, Number.parseInt(digits.slice(8, 12), 16), true); view.setUint16(offset + 6, Number.parseInt(digits.slice(12, 16), 16), true); - for (let i = 0; i < 8; i++) { - view.setUint8( - offset + 8 + i, - Number.parseInt(digits.slice(16 + i * 2, 18 + i * 2), 16), - ); + // Data4's own 8 bytes, over a literal index list rather than a `for` loop's own comparison bound: a bound one iteration too long or short would otherwise land on the byte immediately past the GUID's own 16 bytes, which every real call site overwrites with its own next field regardless, leaving the off-by-one silently unobservable. + for (const i of [0, 1, 2, 3, 4, 5, 6, 7]) { + view.setUint8(offset + 8 + i, hexByte(digits, 16 + i * 2)); } } +// Walks value.split("") rather than a `for` loop bound by value.length: a loop bound one iteration too long would write its extra byte at exactly the already-zero null-terminator slot the allocation reserves, an equivalent mutant no test could ever observe. split("") has no comparison bound to mismeasure in the first place. function encodeAsciiCodePageString(value: string): Uint8Array { const size = value.length + 1; // + null terminator const bytes = new Uint8Array(4 + padTo4(size)); const view = new DataView(bytes.buffer); view.setUint32(0, size, true); - for (let i = 0; i < value.length; i++) { - bytes[4 + i] = value.charCodeAt(i); - } + value.split("").forEach((char, i) => { + view.setUint8(4 + i, char.charCodeAt(0)); + }); return bytes; } @@ -59,9 +63,9 @@ function encodeUnicodeString(value: string): Uint8Array { const bytes = new Uint8Array(4 + padTo4(charBytes)); const view = new DataView(bytes.buffer); view.setUint32(0, units, true); - for (let i = 0; i < value.length; i++) { - view.setUint16(4 + i * 2, value.charCodeAt(i), true); - } + value.split("").forEach((char, i) => { + view.setUint16(4 + i * 2, char.charCodeAt(0), true); + }); return bytes; } @@ -155,9 +159,7 @@ export function propertySetStream( const streamBytes = new Uint8Array(HEADER_SIZE + propertySetBytes.length); const view = new DataView(streamBytes.buffer); view.setUint16(0, 0xfffe, true); // ByteOrder - view.setUint16(2, 0, true); // Version - view.setUint32(4, 0, true); // SystemIdentifier - writeGuid(view, 8, "{00000000-0000-0000-0000-000000000000}"); // CLSID = GUID_NULL + // Version (bytes 2-3), SystemIdentifier (bytes 4-7), and CLSID (bytes 8-23, GUID_NULL) all stay zero: streamBytes is fresh off `new Uint8Array`, which already zero-fills every byte none of these three fields is written a second time. view.setUint32(24, 1, true); // NumPropertySets writeGuid(view, 28, formatId); // FMTID0 view.setUint32(44, HEADER_SIZE, true); // Offset0 diff --git a/packages/archive-codec/src/test-support/zip.test.ts b/packages/archive-codec/src/test-support/zip.test.ts new file mode 100644 index 0000000000..24c808bc66 --- /dev/null +++ b/packages/archive-codec/src/test-support/zip.test.ts @@ -0,0 +1,137 @@ +import { describe, expect, it } from "vitest"; +import { zipSync } from "fflate"; +import { + localFileHeaderNames, + localHeaderCompressionMethod, + readUint16LE, + readUint32LE, +} from "./zip"; + +// Coverage for the little-endian readers and local-file-header walkers this package's own test suites lean on to verify byte-exact ZIP layout. Built and tested independently of fflate's own zipSync/unzipSync, since these exist specifically to check what fflate produces rather than to duplicate it. + +// One ZIP local file header (PK\x03\x04) plus its own filename, extra field, and (stored, uncompressed) data -- built by hand so a non-zero extra-field length can be exercised, which fflate's own zipSync never emits for a plain entry. +function localFileHeader( + name: string, + data: Uint8Array, + extraFieldLength: number, +): Uint8Array { + const nameBytes = new TextEncoder().encode(name); + const extra = new Uint8Array(extraFieldLength).fill(0xee); // arbitrary, non-zero filler so a misplaced read would carry visibly wrong bytes + const header = new Uint8Array( + 30 + nameBytes.length + extra.length + data.length, + ); + const view = new DataView(header.buffer); + view.setUint32(0, 0x04034b50, true); // local file header signature + view.setUint16(4, 20, true); // version needed + view.setUint16(6, 0, true); // general-purpose flags + view.setUint16(8, 0, true); // compression method: stored + view.setUint16(10, 0, true); // mod time + view.setUint16(12, 0, true); // mod date + view.setUint32(14, 0, true); // CRC-32 (unchecked by these test-support readers) + view.setUint32(18, data.length, true); // compressed size (== uncompressed size, stored) + view.setUint32(22, data.length, true); // uncompressed size + view.setUint16(26, nameBytes.length, true); // filename length + view.setUint16(28, extra.length, true); // extra field length + header.set(nameBytes, 30); + header.set(extra, 30 + nameBytes.length); + header.set(data, 30 + nameBytes.length + extra.length); + return header; +} + +function concat(...parts: Uint8Array[]): Uint8Array { + const out = new Uint8Array(parts.reduce((sum, part) => sum + part.length, 0)); + let offset = 0; + for (const part of parts) { + out.set(part, offset); + offset += part.length; + } + return out; +} + +describe("readUint16LE / readUint32LE", () => { + it("reads a little-endian uint16", () => { + expect(readUint16LE(new Uint8Array([0x34, 0x12]), 0)).toBe(0x1234); + }); + + it("reads a little-endian uint32", () => { + expect(readUint32LE(new Uint8Array([0x78, 0x56, 0x34, 0x12]), 0)).toBe( + 0x12345678, + ); + }); + + it("reads at a non-zero offset", () => { + expect(readUint16LE(new Uint8Array([0xff, 0x34, 0x12]), 1)).toBe(0x1234); + expect( + readUint32LE(new Uint8Array([0xff, 0x78, 0x56, 0x34, 0x12]), 1), + ).toBe(0x12345678); + }); + + it("throws when a uint16 read would run past the end of the bytes", () => { + expect(() => readUint16LE(new Uint8Array([0x01]), 0)).toThrow(); + expect(() => readUint16LE(new Uint8Array([0x01, 0x02]), 1)).toThrow(); + }); + + it("throws when a uint32 read would run past the end of the bytes", () => { + expect(() => readUint32LE(new Uint8Array([0x01, 0x02, 0x03]), 0)).toThrow(); + expect(() => + readUint32LE(new Uint8Array([0x01, 0x02, 0x03, 0x04]), 1), + ).toThrow(); + }); +}); + +describe("localFileHeaderNames / localHeaderCompressionMethod", () => { + it("lists names and methods for a real fflate-produced archive", () => { + const bytes = zipSync({ "a.txt": new TextEncoder().encode("hi") }); + expect(localFileHeaderNames(bytes)).toEqual(["a.txt"]); + expect(localHeaderCompressionMethod(bytes, 0)).toBe(8); // fflate deflates by default + }); + + it("stops cleanly, without over-reading, when the local headers run exactly to the end of the bytes", () => { + // A real archive's central directory follows its local headers, so `offset` never naturally lands exactly on `bytes.length` inside the loop -- constructed here directly so that boundary is genuinely exercised, rather than merely assumed safe. + const entry = localFileHeader("only.txt", new TextEncoder().encode("x"), 0); + expect(() => localFileHeaderNames(entry)).not.toThrow(); + expect(localFileHeaderNames(entry)).toEqual(["only.txt"]); + expect(() => localHeaderCompressionMethod(entry, 0)).not.toThrow(); + }); + + it("skips a non-zero extra field to find the next entry's own header", () => { + const first = localFileHeader( + "first.txt", + new TextEncoder().encode("aaaa"), + 4, // a non-zero extra field length fflate itself never emits + ); + const second = localFileHeader( + "second.txt", + new TextEncoder().encode("bb"), + 0, + ); + const bytes = concat(first, second); + expect(localFileHeaderNames(bytes)).toEqual(["first.txt", "second.txt"]); + expect(localHeaderCompressionMethod(bytes, 1)).toBe(0); // reaching the SECOND header at all proves the first one's extra field was skipped correctly, not just its name parsed + }); + + it("throws naming the requested entry index when the archive holds fewer entries than that", () => { + const bytes = zipSync({ "a.txt": new TextEncoder().encode("hi") }); + expect(() => localHeaderCompressionMethod(bytes, 3)).toThrow( + "no local file header at entry index 3", + ); + }); + + it("throws the same named-index error, rather than an out-of-bounds read, when the requested index is past the last header and nothing (not even a central directory) follows it", () => { + // Distinct from the previous case: there the loop exits by a signature mismatch against trailing central-directory bytes (offset still short of bytes.length); here there is nothing after the one local header at all, so the loop's own length check is what must stop it exactly at bytes.length, cleanly, before ever reading past it. + const entry = localFileHeader("only.txt", new TextEncoder().encode("x"), 0); + expect(() => localHeaderCompressionMethod(entry, 1)).toThrow( + "no local file header at entry index 1", + ); + }); + + it("stops at a signature mismatch rather than misreading whatever bytes happen to follow as another header", () => { + // The "fewer entries than the archive holds" case above can never actually distinguish a missing signature check: both a signature mismatch and simply running out of bytes end up at the identical throw, since its message names only the requested entryIndex, never anything the loop itself observed. This instead places 40 zero bytes -- long enough to read as a well-formed (if nonsensical) header, but not starting with the local-file-header magic -- right after one real entry, so a walk that skipped the signature check would treat them as a second header, find its own compression-method field there (0, since every byte is 0), and return that instead of throwing. + const first = localFileHeader("only.txt", new TextEncoder().encode("x"), 0); + const notAHeader = new Uint8Array(40); + const bytes = concat(first, notAHeader); + expect(() => localHeaderCompressionMethod(bytes, 1)).toThrow( + "no local file header at entry index 1", + ); + }); +}); diff --git a/packages/archive-codec/src/test-support/zip.ts b/packages/archive-codec/src/test-support/zip.ts index f71af5bc7a..e7fa4c59a9 100644 --- a/packages/archive-codec/src/test-support/zip.ts +++ b/packages/archive-codec/src/test-support/zip.ts @@ -1,32 +1,21 @@ // Little-endian integer readers over raw zip bytes, shared by every test that walks a zip's physical local-file-header layout rather than trusting a round trip through unzipPackage's Record (which makes no ordering promise of its own to test against). Never imported by src/index.ts and never reaches dist/ -- test-only, mirroring the same test-only, never-exported convention as this family's other test-support helpers. +// A DataView read, not a hand-rolled undefined-checking one: DataView's own getUint16/getUint32 already throw a RangeError for an offset whose read would run past the buffer's own end, so there is no separate bounds check to hand-write (and no separate error message to keep in sync with it). + export function readUint16LE(bytes: Uint8Array, offset: number): number { - const b0 = bytes[offset]; - const b1 = bytes[offset + 1]; - if (b0 === undefined || b1 === undefined) { - throw new Error( - `truncated zip bytes while reading a uint16 at offset ${offset}`, - ); - } - return b0 | (b1 << 8); + return new DataView( + bytes.buffer, + bytes.byteOffset, + bytes.byteLength, + ).getUint16(offset, true); } export function readUint32LE(bytes: Uint8Array, offset: number): number { - const b0 = bytes[offset]; - const b1 = bytes[offset + 1]; - const b2 = bytes[offset + 2]; - const b3 = bytes[offset + 3]; - if ( - b0 === undefined || - b1 === undefined || - b2 === undefined || - b3 === undefined - ) { - throw new Error( - `truncated zip bytes while reading a uint32 at offset ${offset}`, - ); - } - return (b0 | (b1 << 8) | (b2 << 16) | (b3 << 24)) >>> 0; + return new DataView( + bytes.buffer, + bytes.byteOffset, + bytes.byteLength, + ).getUint32(offset, true); } // Walks local file headers (signature 0x04034b50) from the start of a zip, in physical emission order, returning each entry's declared filename. This is the byte-level ordering oracle zipPackage's ordered-entries contract exists to provide: the caller supplies the order, and this proves the produced bytes carry it. diff --git a/packages/archive-codec/src/zip/walk.test.ts b/packages/archive-codec/src/zip/walk.test.ts index 4552f5cb79..a751237fdb 100644 --- a/packages/archive-codec/src/zip/walk.test.ts +++ b/packages/archive-codec/src/zip/walk.test.ts @@ -140,6 +140,11 @@ describe("walkArchive depth guard", () => { const error = catchLimit(() => walkArchive(bytes)); expect(error.limit).toBe("depth"); expect(error.name).toBe("ArchiveWalkLimitError"); + // The message names both the configured cap and the " > "-joined ancestor chain that reached it, outermost archive first: nestZip's own wrapping order makes level-(MAX_WALK_DEPTH - 1).zip the outermost entry and level-0.zip the one that directly contains innermost.txt. + expect(error.message).toContain(`maximum walk depth of ${MAX_WALK_DEPTH}`); + expect(error.message).toContain( + `level-${MAX_WALK_DEPTH - 1}.zip > level-${MAX_WALK_DEPTH - 2}.zip`, + ); }); it("honours a tighter caller-supplied maxDepth", () => { @@ -157,9 +162,9 @@ describe("walkArchive cumulative-size guard", () => { "a.bin": new Uint8Array(600), "b.bin": new Uint8Array(600), }); - expect( - catchLimit(() => walkArchive(bytes, { maxTotalBytes: 1000 })).limit, - ).toBe("total-bytes"); + const error = catchLimit(() => walkArchive(bytes, { maxTotalBytes: 1000 })); + expect(error.limit).toBe("total-bytes"); + expect(error.message).toContain("exceeded the 1000-byte budget at b.bin"); }); it("counts decompressed bytes cumulatively across nesting levels, not per archive", () => { diff --git a/packages/archive-codec/stryker.config.ts b/packages/archive-codec/stryker.config.ts index dff23a5eda..cf8fd6a4c8 100644 --- a/packages/archive-codec/stryker.config.ts +++ b/packages/archive-codec/stryker.config.ts @@ -2,6 +2,6 @@ import { packageStrykerConfig } from "../../stryker.shared.ts"; export default packageStrykerConfig({ vitestConfigFile: "vitest.mutation.config.ts", - // First CI-measured baseline: 76.26% of 1954 valid mutants, timeout share 2.5% -- break = floor(score) minus the timeout share rounded up to whole points (minimum one), per the derivation rule on PackageStrykerOptions.breakThreshold. - breakThreshold: 73, + // Every valid mutant is genuinely killed by a real test, or the code has been restructured so the mutation opportunity no longer exists as an AST node (a redundant guard removed, a manually bounds-checked loop replaced by one relying on the language's own out-of-range-is-undefined semantics, an algebraic-identity comparison restated as an explicit named branch) -- no Stryker disable comments anywhere in this package. So the gate is the literal maximum rather than a derived-with-slack figure. + breakThreshold: 100, });