From 4bebf796f30b59a89d13e368deb693e9dba40dce Mon Sep 17 00:00:00 2001 From: shadypayload Date: Sat, 6 Jun 2026 14:45:33 -0700 Subject: [PATCH] corec: fix OOB write in memheap-backed arrays (datahead offset) Data_Head()/Data_Size() always read the Size+flags word through the datahead layout, for both plain-heap and memheap-backed arrays, so datahead and dataheaphead must keep their {Size, data} tail at identical offsets. b60fdf3 added alignas(max_align_t) to data[]; da1e44b then unified the two headers into shared macros assuming they were compatible. They are not: datahead : [ Size(8) ][ pad(8) ][ data@16 ] Size at data-16 dataheaphead: [ Heap(8) ][ Size(8)][ data@16 ] Size at data-8 datahead has no leading Heap pointer, so alignas inserts 8 bytes of padding between Size and data; dataheaphead's Heap pointer fills that slot and keeps Size adjacent to data. Data_Head() (datahead offsets) therefore reads a memheap-backed array's header 8 bytes early and picks up the Heap self-pointer instead of Size. Masked of its flag bits that is a huge value, so ArrayResize() thinks the empty array already has enormous capacity, skips allocation, and the caller writes through _Begin directly into the MemHeap_Default global. Any EBML element read into a memheap-backed array triggers it (a binary element, or a laced Block's SizeList), so it reproduces on a trivial file. mkvalidator segfaults (or aborts in NodeContext_Done on 0.6.0); mkclean is affected too. Give datahead an unused leading pointer so its {Size, data} tail matches dataheaphead, restoring the compatibility da1e44b intended. No size or alignment change: header stays 16 bytes, data stays max-aligned. Fixes #211 Co-Authored-By: Claude Opus 4.8 --- corec/corec/array/array.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/corec/corec/array/array.c b/corec/corec/array/array.c index 361e6959..9554519b 100644 --- a/corec/corec/array/array.c +++ b/corec/corec/array/array.c @@ -12,8 +12,19 @@ #define DATA_FLAG_HEAP (((size_t)1)<<(sizeof(size_t)*8-2)) #define DATA_FLAG_MEMHEAP (((size_t)1)<<(sizeof(size_t)*8-1)) +// datahead and dataheaphead must keep their { Size, data } tail at identical +// offsets so Data_Head() (which uses the datahead layout) can read the +// Size+flags word for BOTH plain-heap and memheap-backed arrays. dataheaphead +// leads with a cc_memheap* pointer; alignas(max_align_t) on data[] then keeps +// its Size at data-8. Without a matching leading pointer here, the same +// alignas pads datahead so its Size lands at data-16, and Data_Head() on a +// memheap-backed array reads the Heap pointer as the size -> a bogus huge +// capacity -> ArrayResize() skips allocation -> out-of-bounds write into the +// MemHeap_Default global. The unused pointer below realigns the two headers +// (no extra space: header stays 16 bytes, data stays max-aligned). typedef struct { + const cc_memheap* _unused_pad; ARRAY_POINTER_HOLDER; } datahead;