From 78b0bc1ced782ea1d7275be8705e04881b6f63d6 Mon Sep 17 00:00:00 2001 From: silverweed Date: Mon, 13 Jul 2026 13:44:44 +0200 Subject: [PATCH] [tree] Fix TBasket out of bounds write of fEntryOffset Following 15d2d71a6729a804ce7d16fd525ab27225ccc519, fEntryOffset is (conditionally) allocated with a capacity of fNevBuf, i.e. the number of elements that are read into it. However, one code path in WriteBuffer() writes fEntryOffset up to one past the fNevBuf-th element, causing an OOB write. This was (usually) not happening prior to the aforementioned commit because fEntryOffset was allocated with a default capacity of 1000 (even though all elements past the fNevBuf-th were never initialized). To fix the OOB write, we now allocate one extra element when reading fEntryOffset from disk so that the WriteBuffer write is not OOB anymore. --- tree/tree/src/TBasket.cxx | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tree/tree/src/TBasket.cxx b/tree/tree/src/TBasket.cxx index fd17cc22a9c74..8a25921edb57b 100644 --- a/tree/tree/src/TBasket.cxx +++ b/tree/tree/src/TBasket.cxx @@ -1046,19 +1046,26 @@ void TBasket::Streamer(TBuffer &b) if (fNevBuf) { // Alas, ReadArray will read the number of elements to store into fEntryOffset from the file, but it // has no way of knowing whether we're passing a large-enough array. - // Therefore we prevent the problem altogether by ignoring fNevBufSize and just having ReadArray allocate - // the buffer for us. This way we are sure that it will be of the correct size even if the file contains - // corrupted data. - fEntryOffset = nullptr; - auto nElemsRead = b.ReadArray(fEntryOffset); - if (nElemsRead != fNevBuf) { + // Therefore we prevent the problem altogether by ignoring fNevBufSize and peeking into the buffer to + // know the number of elements to allocate beforehand. + // This way we are sure that fEntryOffset will be of the correct size even if the file contains corrupted + // data. + Int_t entryOffsetSize; + auto buf = b.GetCurrent(); + frombuf(buf, &entryOffsetSize); + if (entryOffsetSize != fNevBuf) { Error("Streamer", "Inconsistent length for the entry offset buffer (expected %d elements, read %d). The basket is " "corrupted and not usable.", - fNevBufSize, nElemsRead); + fNevBuf, entryOffsetSize); MakeZombie(); return; } + // We need to allocate an extra element due to the offset/size conversion happening during writing + // (see WriteBuffer) + fEntryOffset = new Int_t[fNevBuf + 1]; + [[maybe_unused]] auto nElemsRead = b.ReadArray(fEntryOffset); + assert(nElemsRead == entryOffsetSize); fNevBufSize = fNevBuf; } else { fEntryOffset = new Int_t[fNevBufSize];