diff --git a/core/cont/src/TArrayC.cxx b/core/cont/src/TArrayC.cxx index c54f6d6399276..88fd28a568194 100644 --- a/core/cont/src/TArrayC.cxx +++ b/core/cont/src/TArrayC.cxx @@ -15,6 +15,7 @@ Array of chars or bytes (8 bits per element). */ #include "TArrayC.h" +#include "ROOT/CheckCollectionSize.hxx" #include "TBuffer.h" @@ -149,6 +150,8 @@ void TArrayC::Streamer(TBuffer &b) if (b.IsReading()) { Int_t n; b >> n; + // Each element occupies at least one byte on file. + n = ROOT::Internal::CheckCollectionSize(b, n, "TArrayC::Streamer", "TArrayC", [] { return true; }); Set(n); b.ReadFastArray(fArray,n); } else { diff --git a/core/cont/src/TArrayD.cxx b/core/cont/src/TArrayD.cxx index bea84d4ac5eaa..9815987aec6fd 100644 --- a/core/cont/src/TArrayD.cxx +++ b/core/cont/src/TArrayD.cxx @@ -15,6 +15,7 @@ Array of doubles (64 bits per element). */ #include "TArrayD.h" +#include "ROOT/CheckCollectionSize.hxx" #include "TBuffer.h" @@ -150,6 +151,8 @@ void TArrayD::Streamer(TBuffer &b) if (b.IsReading()) { Int_t n; b >> n; + // Each element occupies at least one byte on file. + n = ROOT::Internal::CheckCollectionSize(b, n, "TArrayD::Streamer", "TArrayD", [] { return true; }); Set(n); b.ReadFastArray(fArray,n); } else { diff --git a/core/cont/src/TArrayF.cxx b/core/cont/src/TArrayF.cxx index 59d76a9488143..e0f60b1848b5f 100644 --- a/core/cont/src/TArrayF.cxx +++ b/core/cont/src/TArrayF.cxx @@ -15,6 +15,7 @@ Array of floats (32 bits per element). */ #include "TArrayF.h" +#include "ROOT/CheckCollectionSize.hxx" #include "TBuffer.h" @@ -149,6 +150,8 @@ void TArrayF::Streamer(TBuffer &b) if (b.IsReading()) { Int_t n; b >> n; + // Each element occupies at least one byte on file. + n = ROOT::Internal::CheckCollectionSize(b, n, "TArrayF::Streamer", "TArrayF", [] { return true; }); Set(n); b.ReadFastArray(fArray,n); } else { diff --git a/core/cont/src/TArrayI.cxx b/core/cont/src/TArrayI.cxx index f17c8b74be16a..7c4b658a8f136 100644 --- a/core/cont/src/TArrayI.cxx +++ b/core/cont/src/TArrayI.cxx @@ -15,6 +15,7 @@ Array of integers (32 bits per element). */ #include "TArrayI.h" +#include "ROOT/CheckCollectionSize.hxx" #include "TBuffer.h" @@ -149,6 +150,8 @@ void TArrayI::Streamer(TBuffer &b) if (b.IsReading()) { Int_t n; b >> n; + // Each element occupies at least one byte on file. + n = ROOT::Internal::CheckCollectionSize(b, n, "TArrayI::Streamer", "TArrayI", [] { return true; }); Set(n); b.ReadFastArray(fArray,n); } else { diff --git a/core/cont/src/TArrayL.cxx b/core/cont/src/TArrayL.cxx index a4480ad4b7bfe..e20ad4f067b83 100644 --- a/core/cont/src/TArrayL.cxx +++ b/core/cont/src/TArrayL.cxx @@ -15,6 +15,7 @@ Array of longs (32 or 64 bits per element). */ #include "TArrayL.h" +#include "ROOT/CheckCollectionSize.hxx" #include "TBuffer.h" @@ -149,6 +150,8 @@ void TArrayL::Streamer(TBuffer &b) if (b.IsReading()) { Int_t n; b >> n; + // Each element occupies at least one byte on file. + n = ROOT::Internal::CheckCollectionSize(b, n, "TArrayL::Streamer", "TArrayL", [] { return true; }); Set(n); b.ReadFastArray(fArray,n); } else { diff --git a/core/cont/src/TArrayL64.cxx b/core/cont/src/TArrayL64.cxx index 3ae8427811ce9..415636a1f9106 100644 --- a/core/cont/src/TArrayL64.cxx +++ b/core/cont/src/TArrayL64.cxx @@ -15,6 +15,7 @@ Array of long64s (64 bits per element). */ #include "TArrayL64.h" +#include "ROOT/CheckCollectionSize.hxx" #include "TBuffer.h" @@ -149,6 +150,8 @@ void TArrayL64::Streamer(TBuffer &b) if (b.IsReading()) { Int_t n; b >> n; + // Each element occupies at least one byte on file. + n = ROOT::Internal::CheckCollectionSize(b, n, "TArrayL64::Streamer", "TArrayL64", [] { return true; }); Set(n); b.ReadFastArray(fArray,n); } else { diff --git a/core/cont/src/TArrayS.cxx b/core/cont/src/TArrayS.cxx index a00dad0146c9a..25595bba73805 100644 --- a/core/cont/src/TArrayS.cxx +++ b/core/cont/src/TArrayS.cxx @@ -15,6 +15,7 @@ Array of shorts (16 bits per element). */ #include "TArrayS.h" +#include "ROOT/CheckCollectionSize.hxx" #include "TBuffer.h" @@ -149,6 +150,8 @@ void TArrayS::Streamer(TBuffer &b) if (b.IsReading()) { Int_t n; b >> n; + // Each element occupies at least one byte on file. + n = ROOT::Internal::CheckCollectionSize(b, n, "TArrayS::Streamer", "TArrayS", [] { return true; }); Set(n); b.ReadFastArray(fArray,n); } else { diff --git a/core/cont/src/TClonesArray.cxx b/core/cont/src/TClonesArray.cxx index 6d464ffa81aec..c615666d40e90 100644 --- a/core/cont/src/TClonesArray.cxx +++ b/core/cont/src/TClonesArray.cxx @@ -131,8 +131,10 @@ When investigating misuse of TClonesArray, please make sure of the following: #include "TClass.h" #include "TObject.h" #include "TObjectTable.h" +#include "ROOT/CheckCollectionSize.hxx" #include +#include #include @@ -779,9 +781,16 @@ void TClonesArray::Streamer(TBuffer &b) } b >> nobjects; - if (nobjects < 0) + if (nobjects < 0 && nobjects != std::numeric_limits::min()) nobjects = -nobjects; // still there for backward compatibility b >> fLowerBound; + // Without bypassing the streamer, each object starts with a byte telling whether it is present. Otherwise, the + // objects are read below with the layout of fClass, which stays the target class if the user changed it. + nobjects = ROOT::Internal::CheckCollectionSize(b, nobjects, "TClonesArray::Streamer", "TClonesArray", [&] { + TClass *readClass = fClass && classv != fClass->GetName() ? fClass : cl; + return !CanBypassStreamer() || b.TestBit(TBuffer::kCannotHandleMemberWiseStreaming) || + ROOT::Internal::MemberWiseElementsUseBytes(readClass->GetStreamerInfo(clv)); + }); if (!fClass) { fClass = cl; if (!fKeep) { diff --git a/core/cont/src/TObjArray.cxx b/core/cont/src/TObjArray.cxx index 804de770168e9..b5d3b2d299c12 100644 --- a/core/cont/src/TObjArray.cxx +++ b/core/cont/src/TObjArray.cxx @@ -52,6 +52,7 @@ set to the owner of its own content. #include "TROOT.h" #include "TBuffer.h" #include "TVirtualMutex.h" +#include "ROOT/CheckCollectionSize.hxx" #include @@ -460,6 +461,9 @@ void TObjArray::Streamer(TBuffer &b) b >> nobjects; b >> fLowerBound; + // Each object occupies at least one byte on file, as it is stored as a pointer, i.e. a tag of 4 bytes. + nobjects = + ROOT::Internal::CheckCollectionSize(b, nobjects, "TObjArray::Streamer", "TObjArray", [] { return true; }); if (nobjects >= fSize) Expand(nobjects); fLast = -1; TObject *obj; diff --git a/core/cont/src/TRefArray.cxx b/core/cont/src/TRefArray.cxx index 8c6c56e16d3dc..f5b1d461d1254 100644 --- a/core/cont/src/TRefArray.cxx +++ b/core/cont/src/TRefArray.cxx @@ -99,6 +99,7 @@ Fatal in : Maximum number of TProcessID (65535) has be #include "TRefTable.h" #include "TBuffer.h" #include "TError.h" +#include "ROOT/CheckCollectionSize.hxx" #include "TBits.h" @@ -524,6 +525,9 @@ void TRefArray::Streamer(TBuffer &R__b) fName.Streamer(R__b); R__b >> nobjects; R__b >> fLowerBound; + // Each object occupies at least one byte on file, as it is stored as its unique ID of 4 bytes. + nobjects = + ROOT::Internal::CheckCollectionSize(R__b, nobjects, "TRefArray::Streamer", "TRefArray", [] { return true; }); if (nobjects >= fSize) Expand(nobjects); fLast = -1; R__b >> pidf; diff --git a/core/meta/CMakeLists.txt b/core/meta/CMakeLists.txt index 8e010343a9a34..efb02a7ae7f76 100644 --- a/core/meta/CMakeLists.txt +++ b/core/meta/CMakeLists.txt @@ -91,6 +91,8 @@ target_sources(Core PRIVATE target_include_directories(Core PUBLIC $ + PRIVATE + res ) ROOT_INSTALL_HEADERS() diff --git a/core/meta/res/ROOT/CheckCollectionSize.hxx b/core/meta/res/ROOT/CheckCollectionSize.hxx new file mode 100644 index 0000000000000..3afea00414dc0 --- /dev/null +++ b/core/meta/res/ROOT/CheckCollectionSize.hxx @@ -0,0 +1,122 @@ +/************************************************************************* + * Copyright (C) 1995-2026, Rene Brun and Fons Rademakers. * + * All rights reserved. * + * * + * For the licensing terms see $ROOTSYS/LICENSE. * + * For the list of contributors see $ROOTSYS/README/CREDITS. * + *************************************************************************/ + +#ifndef ROOT_CheckCollectionSize +#define ROOT_CheckCollectionSize + +// Private header shared by libCore and libRIO. + +#include "RtypesCore.h" +#include "TBuffer.h" +#include "TError.h" +#include "TObjArray.h" +#include "TStreamerElement.h" +#include "TVirtualStreamerInfo.h" + +namespace ROOT { +namespace Internal { + +//////////////////////////////////////////////////////////////////////////////// +/// Validate the number of elements of a collection that was just read from `buf`. +/// +/// The collection size is stored as an Int_t but consumed as an unsigned value +/// (TVirtualCollectionProxy::Allocate, std::vector::resize), so a negative value +/// coming from corrupt data would turn into a request for billions of elements. +/// Report the problem from `location` and read the collection as empty instead. +inline Int_t CheckCollectionSize(TBuffer &buf, Int_t nElements, const char *location, const char *collectionName) +{ + if (R__unlikely(nElements < 0)) { + ::Error(location, + "Corrupt data: the collection of type %s read from %s has a negative number of elements (%d); " + "reading it as empty.", + collectionName ? collectionName : "(unknown)", + buf.GetParent() ? buf.GetParent()->GetName() : "memory/socket", nElements); + return 0; + } + return nElements; +} + +//////////////////////////////////////////////////////////////////////////////// +/// Validate the number of elements of a collection that was just read from `buf`. +/// +/// In addition to the checks of the overload above, reject sizes larger than +/// the number of bytes left in the buffer if each element is known to occupy +/// at least one byte on file. Such a size can only come from corrupt data, and +/// unlike a negative one it is not caught by Allocate or resize, which would +/// try to allocate and initialise up to 2^31 elements. +/// +/// \param[in] elementsUseBytes Callable returning whether each element occupies +/// at least one byte on file. It is only called for sizes larger than +/// the remaining bytes, so it does not slow down reading valid data. +template +inline Int_t CheckCollectionSize(TBuffer &buf, Int_t nElements, const char *location, const char *collectionName, + ElementsUseBytes &&elementsUseBytes) +{ + nElements = CheckCollectionSize(buf, nElements, location, collectionName); + const Long64_t remaining = (Long64_t)buf.BufferSize() - buf.Length(); + // TBufferText and its subclasses do not keep the data in the buffer. TBufferSQL inherits from TBufferFile + // without doing so either, but TTreeSQL does not support branches of collections or objects. + if (R__unlikely(nElements > remaining) && buf.InheritsFrom("TBufferFile") && elementsUseBytes()) { + ::Error(location, + "Corrupt data: the collection of type %s read from %s has %d elements, but only %lld bytes are left " + "in the buffer; reading it as empty.", + collectionName ? collectionName : "(unknown)", + buf.GetParent() ? buf.GetParent()->GetName() : "memory/socket", nElements, remaining); + return 0; + } + return nElements; +} + +//////////////////////////////////////////////////////////////////////////////// +/// Return whether each element of a collection streamed member-wise with the +/// on-file layout `info` occupies at least one byte on file. +/// +/// This is the case as soon as one of the data members is streamed as a +/// basic type or an array of them, a pointer written with an object tag, a +/// string or an STL collection. Members of class type are not considered: an +/// object can occupy zero bytes, e.g. with a custom streamer. Neither are +/// pointers marked `//->`, whose object is streamed in place. +inline bool MemberWiseElementsUseBytes(const TVirtualStreamerInfo *info) +{ + if (!info || !info->GetElements()) + return false; + for (const auto *obj : *info->GetElements()) { + const auto *element = dynamic_cast(obj); + // Artificial elements are set by I/O customization rules, they do not read from the buffer, and custom member + // streamers can read anything. + if (!element || dynamic_cast(element) || element->GetStreamer()) + continue; + const Int_t type = element->GetType(); + switch (type) { + case TVirtualStreamerInfo::kObjectP: + case TVirtualStreamerInfo::kTString: + case TVirtualStreamerInfo::kTObject: + case TVirtualStreamerInfo::kTNamed: + case TVirtualStreamerInfo::kAnyP: + case TVirtualStreamerInfo::kAnyPnoVT: + case TVirtualStreamerInfo::kSTLp: return true; + default: + if (type > TVirtualStreamerInfo::kBase && type < TVirtualStreamerInfo::kObject) { + // A basic type, an array of them (kOffsetL), or a pointer to an array of them (kOffsetP), which starts + // with a byte telling whether the array is present. + const Int_t basicType = type % TVirtualStreamerInfo::kOffsetL; + if (basicType >= TVirtualStreamerInfo::kChar && basicType <= TVirtualStreamerInfo::kFloat16 && + basicType != TVirtualStreamerInfo::kLegacyChar) + return true; + } else if (type >= TVirtualStreamerInfo::kSTL && type <= TVirtualStreamerInfo::kSTLstring) { + return true; + } + } + } + return false; +} + +} // namespace Internal +} // namespace ROOT + +#endif diff --git a/io/io/src/ObjectWiseElementsUseBytes.h b/io/io/src/ObjectWiseElementsUseBytes.h new file mode 100644 index 0000000000000..010da27784fb4 --- /dev/null +++ b/io/io/src/ObjectWiseElementsUseBytes.h @@ -0,0 +1,54 @@ +/************************************************************************* + * Copyright (C) 1995-2026, Rene Brun and Fons Rademakers. * + * All rights reserved. * + * * + * For the licensing terms see $ROOTSYS/LICENSE. * + * For the list of contributors see $ROOTSYS/README/CREDITS. * + *************************************************************************/ + +#ifndef ROOT_ObjectWiseElementsUseBytes +#define ROOT_ObjectWiseElementsUseBytes + +#include "TDataType.h" +#include "TDictionary.h" +#include "TGenCollectionProxy.h" + +namespace ROOT { +namespace Internal { + +//////////////////////////////////////////////////////////////////////////////// +/// Return whether an element described by `value` occupies at least one byte on +/// file when streamed object-wise by a collection proxy. +/// +/// Objects of class type can occupy zero bytes, e.g. with a custom streamer, and +/// pointers to fundamental types are not streamed at all. +inline bool ObjectWiseElementUsesBytes(const TGenCollectionProxy::Value *value) +{ + if (!value) + return false; + switch (value->fCase) { + case kIsFundamental: + case kIsEnum: + // These kinds are not supported, and are not streamed. + return value->fKind != kchar && value->fKind != kNoType_t && value->fKind != kOther_t; + case TGenCollectionProxy::kBIT_ISSTRING: + case 0U | kIsPointer | kIsClass: + case 0U | kIsPointer | TGenCollectionProxy::kBIT_ISSTRING: + case 0U | kIsPointer | TGenCollectionProxy::kBIT_ISTSTRING | kIsClass: return true; + default: return false; + } +} + +//////////////////////////////////////////////////////////////////////////////// +/// Return whether each element of a collection proxy with the given key (for +/// maps, nullptr otherwise) and value descriptors occupies at least one byte on +/// file when streamed object-wise. +inline bool ObjectWiseElementsUseBytes(const TGenCollectionProxy::Value *key, const TGenCollectionProxy::Value *value) +{ + return ObjectWiseElementUsesBytes(key) || ObjectWiseElementUsesBytes(value); +} + +} // namespace Internal +} // namespace ROOT + +#endif diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index a20a67ec60861..2657823f09bc7 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -180,6 +180,28 @@ void TBufferFile::ReadLong(Long_t &l) } } +//////////////////////////////////////////////////////////////////////////////// +/// Return whether the length of a string just read from `buf` can only come +/// from corrupt data, i.e. it is negative or larger than the bytes left in the +/// buffer, and report it from `method`. + +static bool IsCorruptStringLength(const TBufferFile &buf, Int_t length, const char *method) +{ + const Long64_t remaining = (Long64_t)buf.BufferSize() - buf.Length(); + if (R__likely(length >= 0 && length <= remaining)) + return false; + const char *parent = buf.GetParent() ? buf.GetParent()->GetName() : "memory/socket"; + if (length < 0) + buf.Error(method, "Corrupt data: the string read from %s has a negative length (%d); reading it as empty.", + parent, length); + else + buf.Error(method, + "Corrupt data: the string read from %s has a length of %d, but only %lld bytes are left in the " + "buffer; reading it as empty.", + parent, length, remaining); + return true; +} + //////////////////////////////////////////////////////////////////////////////// /// Read TString from TBuffer. @@ -192,10 +214,16 @@ void TBufferFile::ReadTString(TString &s) s.UnLink(); s.Zero(); } else { - if (nwh == 255) + if (nwh == 255) { *this >> nbig; - else + if (IsCorruptStringLength(*this, nbig, "ReadTString")) { + s.UnLink(); + s.Zero(); + return; + } + } else { nbig = nwh; + } nbig = s.Clobber(nbig); // update length since Clobber clamps to MaxSize (if Fatal does not abort) char *data = s.GetPointer(); @@ -245,6 +273,10 @@ void TBufferFile::ReadStdString(std::string *obj) } if (nwh == 255) { *this >> nbig; + if (IsCorruptStringLength(*this, nbig, "ReadStdString")) { + obj->clear(); + return; + } obj->resize(nbig,'\0'); ReadFastArray((char*)obj->data(),nbig); } @@ -294,6 +326,8 @@ void TBufferFile::ReadCharStar(char* &s) Error("ReadCharStar", "Cannot allocate buffer: maximum capacity exceeded (%d bytes)!", std::numeric_limits::max()); return; } + if (IsCorruptStringLength(*this, nch, "ReadCharStar")) + return; s = new char[nch+1]; ReadFastArray(s, nch); s[nch] = 0; diff --git a/io/io/src/TContainerConverters.cxx b/io/io/src/TContainerConverters.cxx index 8e25771e4e812..6cd437c3c4d56 100644 --- a/io/io/src/TContainerConverters.cxx +++ b/io/io/src/TContainerConverters.cxx @@ -31,7 +31,9 @@ #include "TError.h" #include "TGenCollectionStreamer.h" #include "TClassStreamer.h" +#include "ROOT/CheckCollectionSize.hxx" #include +#include namespace { const Int_t kMapOffset = 2; @@ -201,7 +203,14 @@ void TConvertClonesArrayToProxy::operator()(TBuffer &b, void *pmember, Int_t siz } b >> nobjects; - if (nobjects < 0) nobjects = -nobjects; // still there for backward compatibility + if (nobjects < 0 && nobjects != std::numeric_limits::min()) + nobjects = -nobjects; // still there for backward compatibility + // Without kBypassStreamer, each element starts with a byte telling whether it is present. + nobjects = ROOT::Internal::CheckCollectionSize(b, nobjects, "TConvertClonesArrayToProxy::operator()", + fCollectionClass->GetName(), [&] { + return !objdummy.TestBit(TClonesArray::kBypassStreamer) || + ROOT::Internal::MemberWiseElementsUseBytes(subinfo); + }); b >> dummy; // fLowerBound is ignored if (cl != subinfo->GetClass()) { Error("TClonesArray::Conversion to vector","Bad class"); diff --git a/io/io/src/TEmulatedCollectionProxy.cxx b/io/io/src/TEmulatedCollectionProxy.cxx index 883dab626fd3a..b041abdb4fe20 100644 --- a/io/io/src/TEmulatedCollectionProxy.cxx +++ b/io/io/src/TEmulatedCollectionProxy.cxx @@ -35,6 +35,8 @@ the class TEmulatedMapProxy. #include "TVirtualMutex.h" // For R__LOCKGUARD #include "TInterpreter.h" // For gInterpreterMutex +#include "ROOT/CheckCollectionSize.hxx" +#include "ObjectWiseElementsUseBytes.h" // // Utility function to allow the creation of a TClass for a std::pair without @@ -665,6 +667,9 @@ void TEmulatedCollectionProxy::ReadBuffer(TBuffer &b, void *obj) TPushPop env(this,obj); int nElements = 0; b >> nElements; + nElements = + ROOT::Internal::CheckCollectionSize(b, nElements, "TEmulatedCollectionProxy::ReadBuffer", fName.c_str(), + [&] { return ROOT::Internal::ObjectWiseElementsUseBytes(fKey, fVal); }); if ( fEnv->fObject ) { Resize(nElements,true); } @@ -679,6 +684,9 @@ void TEmulatedCollectionProxy::Streamer(TBuffer &b) if ( b.IsReading() ) { //Read mode int nElements = 0; b >> nElements; + nElements = + ROOT::Internal::CheckCollectionSize(b, nElements, "TEmulatedCollectionProxy::Streamer", fName.c_str(), + [&] { return ROOT::Internal::ObjectWiseElementsUseBytes(fKey, fVal); }); if ( fEnv->fObject ) { Resize(nElements,true); } diff --git a/io/io/src/TEmulatedMapProxy.cxx b/io/io/src/TEmulatedMapProxy.cxx index d1ec2d546bd55..90eeb7658f748 100644 --- a/io/io/src/TEmulatedMapProxy.cxx +++ b/io/io/src/TEmulatedMapProxy.cxx @@ -35,6 +35,8 @@ is separated from the TEmulatedProxy. #include "TClassEdit.h" #include "TStreamerInfo.h" #include "TError.h" +#include "ROOT/CheckCollectionSize.hxx" +#include "ObjectWiseElementsUseBytes.h" TEmulatedMapProxy::TEmulatedMapProxy(const TEmulatedMapProxy& copy) : TEmulatedCollectionProxy(copy) @@ -226,6 +228,9 @@ void TEmulatedMapProxy::ReadBuffer(TBuffer &b, void *obj) TPushPop env(this,obj); int nElements = 0; b >> nElements; + nElements = ROOT::Internal::CheckCollectionSize(b, nElements, "TEmulatedMapProxy::ReadBuffer", fName.c_str(), [&] { + return ROOT::Internal::ObjectWiseElementsUseBytes(fKey, fVal); + }); if ( fEnv->fObject ) { Resize(nElements,true); } @@ -238,8 +243,11 @@ void TEmulatedMapProxy::Streamer(TBuffer &b) { // TClassStreamer IO overload. if ( b.IsReading() ) { //Read mode - UInt_t nElements = 0; + Int_t nElements = 0; b >> nElements; + nElements = ROOT::Internal::CheckCollectionSize(b, nElements, "TEmulatedMapProxy::Streamer", fName.c_str(), [&] { + return ROOT::Internal::ObjectWiseElementsUseBytes(fKey, fVal); + }); if ( fEnv->fObject ) { Resize(nElements,true); } diff --git a/io/io/src/TGenCollectionStreamer.cxx b/io/io/src/TGenCollectionStreamer.cxx index b8918ec5de02a..eb1c990256847 100644 --- a/io/io/src/TGenCollectionStreamer.cxx +++ b/io/io/src/TGenCollectionStreamer.cxx @@ -29,6 +29,8 @@ size(), clear(), resize(). resize() may be a void operation. #include "TStreamerInfo.h" #include "TStreamerElement.h" #include "TVirtualCollectionIterators.h" +#include "ROOT/CheckCollectionSize.hxx" +#include "ObjectWiseElementsUseBytes.h" #include @@ -1183,6 +1185,8 @@ void TGenCollectionStreamer::ReadBufferVectorPrimitives(TBuffer &b, void *obj, c { int nElements = 0; b >> nElements; + nElements = ROOT::Internal::CheckCollectionSize(b, nElements, "TGenCollectionStreamer::ReadBufferVectorPrimitives", + fName.c_str(), [] { return true; }); fResize(obj,nElements); if (onFileClass) { @@ -1198,6 +1202,8 @@ void TGenCollectionStreamer::ReadBufferVectorPrimitivesFloat16(TBuffer &b, void { int nElements = 0; b >> nElements; + nElements = ROOT::Internal::CheckCollectionSize( + b, nElements, "TGenCollectionStreamer::ReadBufferVectorPrimitivesFloat16", fName.c_str(), [] { return true; }); fResize(obj,nElements); if (onFileClass) { @@ -1213,6 +1219,8 @@ void TGenCollectionStreamer::ReadBufferVectorPrimitivesDouble32(TBuffer &b, void { int nElements = 0; b >> nElements; + nElements = ROOT::Internal::CheckCollectionSize( + b, nElements, "TGenCollectionStreamer::ReadBufferVectorPrimitivesDouble32", fName.c_str(), [] { return true; }); fResize(obj,nElements); if (onFileClass) { @@ -1318,6 +1326,9 @@ void TGenCollectionStreamer::ReadBufferGeneric(TBuffer &b, void *obj, const TCla int nElements = 0; b >> nElements; + nElements = + ROOT::Internal::CheckCollectionSize(b, nElements, "TGenCollectionStreamer::ReadBufferGeneric", fName.c_str(), + [&] { return ROOT::Internal::ObjectWiseElementsUseBytes(fKey, fVal); }); if (nElements == 0) { if (obj) { @@ -1402,6 +1413,9 @@ void TGenCollectionStreamer::Streamer(TBuffer &b) if (b.IsReading()) { //Read mode int nElements = 0; b >> nElements; + nElements = + ROOT::Internal::CheckCollectionSize(b, nElements, "TGenCollectionStreamer::Streamer", fName.c_str(), + [&] { return ROOT::Internal::ObjectWiseElementsUseBytes(fKey, fVal); }); if (fEnv->fObject) { TGenCollectionProxy::Clear("force"); } @@ -1480,6 +1494,9 @@ void TGenCollectionStreamer::StreamerAsMap(TBuffer &b) if (b.IsReading()) { //Read mode int nElements = 0; b >> nElements; + nElements = + ROOT::Internal::CheckCollectionSize(b, nElements, "TGenCollectionStreamer::StreamerAsMap", fName.c_str(), + [&] { return ROOT::Internal::ObjectWiseElementsUseBytes(fKey, fVal); }); if (fEnv->fObject) { TGenCollectionProxy::Clear("force"); } diff --git a/io/io/src/TStreamerInfoActions.cxx b/io/io/src/TStreamerInfoActions.cxx index 08468706c7f67..b16ed276aa253 100644 --- a/io/io/src/TStreamerInfoActions.cxx +++ b/io/io/src/TStreamerInfoActions.cxx @@ -22,6 +22,7 @@ #include "TMemberStreamer.h" #include "TClassEdit.h" #include "TVirtualCollectionIterators.h" +#include "ROOT/CheckCollectionSize.hxx" #include "TProcessID.h" #include "TFile.h" @@ -784,6 +785,11 @@ namespace TStreamerInfoActions TVirtualCollectionProxy::TPushPop helper( oldProxy, (char*)addr ); Int_t nobjects; buf.ReadInt(nobjects); + nobjects = ROOT::Internal::CheckCollectionSize( + buf, nobjects, "TStreamerInfoActions::ReadSTLMemberWiseSameClass", config->fTypeName, [&] { + auto *memberWise = oldProxy->GetReadMemberWiseActions(vClVersion); + return memberWise && ROOT::Internal::MemberWiseElementsUseBytes(memberWise->fStreamerInfo); + }); void* alternative = oldProxy->Allocate(nobjects,true); if (nobjects) { TActionSequence *actions = oldProxy->GetReadMemberWiseActions( vClVersion ); @@ -816,6 +822,11 @@ namespace TStreamerInfoActions TVirtualCollectionProxy::TPushPop helper( oldProxy, (char*)addr ); Int_t nobjects; buf.ReadInt(nobjects); + nobjects = ROOT::Internal::CheckCollectionSize( + buf, nobjects, "TStreamerInfoActions::ReadSTLMemberWiseSameClass", config->fTypeName, [&] { + TClass *valueClass = oldProxy->GetValueClass(); + return valueClass && ROOT::Internal::MemberWiseElementsUseBytes(valueClass->GetStreamerInfo(0)); + }); void* env = oldProxy->Allocate(nobjects,true); if (nobjects || vers < 7 ) { @@ -856,6 +867,9 @@ namespace TStreamerInfoActions for(; objfTypeName, + [&] { return actions && ROOT::Internal::MemberWiseElementsUseBytes(actions->fStreamerInfo); }); TVirtualCollectionProxy::TPushPop helper( oldProxy, (char*)obj ); void* alternative = oldProxy->Allocate(nobjects,true); if (nobjects) { @@ -893,6 +907,11 @@ namespace TStreamerInfoActions TVirtualCollectionProxy::TPushPop helper( oldProxy, (char*)obj ); Int_t nobjects; buf.ReadInt(nobjects); + nobjects = ROOT::Internal::CheckCollectionSize( + buf, nobjects, "TStreamerInfoActions::ReadArraySTLMemberWiseSameClass", config->fTypeName, [&] { + TClass *valueClass = oldProxy->GetValueClass(); + return valueClass && ROOT::Internal::MemberWiseElementsUseBytes(valueClass->GetStreamerInfo(0)); + }); void* env = oldProxy->Allocate(nobjects,true); if (nobjects || vers < 7 ) { @@ -930,6 +949,11 @@ namespace TStreamerInfoActions TVirtualCollectionProxy::TPushPop helper( newProxy, (char*)addr ); Int_t nobjects; buf.ReadInt(nobjects); + nobjects = ROOT::Internal::CheckCollectionSize( + buf, nobjects, "TStreamerInfoActions::ReadSTLMemberWiseChangedClass", config->fTypeName, [&] { + auto *conversion = newProxy->GetConversionReadMemberWiseActions(oldProxy->GetValueClass(), vClVersion); + return conversion && ROOT::Internal::MemberWiseElementsUseBytes(conversion->fStreamerInfo); + }); void* alternative = newProxy->Allocate(nobjects,true); if (nobjects) { TActionSequence *actions = newProxy->GetConversionReadMemberWiseActions( oldProxy->GetValueClass(), vClVersion ); @@ -979,6 +1003,12 @@ namespace TStreamerInfoActions TVirtualCollectionProxy::TPushPop helper( newProxy, (char*)obj ); Int_t nobjects; buf.ReadInt(nobjects); + nobjects = ROOT::Internal::CheckCollectionSize( + buf, nobjects, "TStreamerInfoActions::ReadArraySTLMemberWiseChangedClass", config->fTypeName, [&] { + auto *conversion = + newProxy->GetConversionReadMemberWiseActions(oldProxy->GetValueClass(), vClVersion); + return conversion && ROOT::Internal::MemberWiseElementsUseBytes(conversion->fStreamerInfo); + }); void* alternative = newProxy->Allocate(nobjects,true); if (nobjects) { TActionSequence *actions = newProxy->GetConversionReadMemberWiseActions( oldProxy->GetValueClass(), vClVersion ); @@ -2232,6 +2262,8 @@ namespace TStreamerInfoActions std::vector *const vec = (std::vector*)(((char*)addr)+config->fOffset); Int_t nvalues; buf.ReadInt(nvalues); + nvalues = ROOT::Internal::CheckCollectionSize(buf, nvalues, "TStreamerInfoActions::ReadCollectionBasicType", + config->fTypeName, [] { return true; }); vec->resize(nvalues); #ifdef R__VISUAL_CPLUSPLUS @@ -2277,6 +2309,8 @@ namespace TStreamerInfoActions std::vector *const vec = (std::vector*)(((char*)addr)+config->fOffset); Int_t nvalues; buf.ReadInt(nvalues); + nvalues = ROOT::Internal::CheckCollectionSize(buf, nvalues, "TStreamerInfoActions::ReadCollectionFloat16", + config->fTypeName, [] { return true; }); vec->resize(nvalues); #ifdef R__VISUAL_CPLUSPLUS @@ -2321,6 +2355,8 @@ namespace TStreamerInfoActions std::vector *const vec = (std::vector*)(((char*)addr)+config->fOffset); Int_t nvalues; buf.ReadInt(nvalues); + nvalues = ROOT::Internal::CheckCollectionSize(buf, nvalues, "TStreamerInfoActions::ReadCollectionDouble32", + config->fTypeName, [] { return true; }); vec->resize(nvalues); #ifdef R__VISUAL_CPLUSPLUS @@ -2367,6 +2403,9 @@ namespace TStreamerInfoActions std::vector *const vec = (std::vector*)(((char*)addr)+config->fOffset); Int_t nvalues; buf.ReadInt(nvalues); + nvalues = + ROOT::Internal::CheckCollectionSize(buf, nvalues, "TStreamerInfoActions::ConvertCollectionBasicType", + config->fTypeName, [] { return true; }); vec->resize(nvalues); From *temp = new From[nvalues]; @@ -2394,6 +2433,9 @@ namespace TStreamerInfoActions std::vector *const vec = (std::vector*)(((char*)addr)+config->fOffset); Int_t nvalues; buf.ReadInt(nvalues); + nvalues = + ROOT::Internal::CheckCollectionSize(buf, nvalues, "TStreamerInfoActions::ConvertCollectionBasicType", + config->fTypeName, [] { return true; }); vec->resize(nvalues); From *temp = new From[nvalues]; @@ -2420,6 +2462,8 @@ namespace TStreamerInfoActions std::vector *const vec = (std::vector*)(((char*)addr)+config->fOffset); Int_t nvalues; buf.ReadInt(nvalues); + nvalues = ROOT::Internal::CheckCollectionSize(buf, nvalues, "TStreamerInfoActions::ConvertCollectionDouble32", + config->fTypeName, [] { return true; }); vec->resize(nvalues); Double32_t *temp = new Double32_t[nvalues]; @@ -2766,6 +2810,8 @@ namespace TStreamerInfoActions Int_t nvalues; buf.ReadInt(nvalues); + nvalues = ROOT::Internal::CheckCollectionSize(buf, nvalues, "TStreamerInfoActions::ReadNumericalCollection", + config->fTypeName, [] { return true; }); void* alternative = newProxy->Allocate(nvalues,true); if (nvalues) { char startbuf[TVirtualCollectionProxy::fgIteratorArenaSize]; @@ -3364,6 +3410,8 @@ namespace TStreamerInfoActions Int_t nvalues; buf.ReadInt(nvalues); + nvalues = ROOT::Internal::CheckCollectionSize(buf, nvalues, "TStreamerInfoActions::ReadNumericalCollection", + config->fTypeName, [] { return true; }); void* alternative = newProxy->Allocate(nvalues,true); if (nvalues) { char startbuf[TVirtualCollectionProxy::fgIteratorArenaSize]; diff --git a/io/io/src/TStreamerInfoReadBuffer.cxx b/io/io/src/TStreamerInfoReadBuffer.cxx index 8135f9416fc82..8e9cdf9b6e619 100644 --- a/io/io/src/TStreamerInfoReadBuffer.cxx +++ b/io/io/src/TStreamerInfoReadBuffer.cxx @@ -25,6 +25,7 @@ #include "TVirtualArray.h" #include "TVirtualObject.h" #include "ThreadLocalStorage.h" +#include "ROOT/CheckCollectionSize.hxx" // GetCurrentElement. // Currently only used by TRef::Streamer. @@ -258,12 +259,9 @@ Int_t TStreamerInfo::ReadBufferSkip(TBuffer &b, const T &arr, const TCompInfo *c // skip char* case TStreamerInfo::kSkip + TStreamerInfo::kCharStar: { DOLOOP { - Int_t nch; b >> nch; - if (nch>0) { - char* readbuf = new char[nch]; - b.ReadFastArray(readbuf,nch); - delete[] readbuf; - } + char *readbuf = nullptr; + b.ReadCharStar(readbuf); + delete[] readbuf; } break; } @@ -1193,6 +1191,9 @@ Int_t TStreamerInfo::ReadBuffer(TBuffer &b, const T &arr, TVirtualCollectionProxy::TPushPop helper( newProxy, cont ); Int_t nobjects; b >> nobjects; + nobjects = ROOT::Internal::CheckCollectionSize( + b, nobjects, "TStreamerInfo::ReadBuffer", cle->GetName(), + [&] { return ROOT::Internal::MemberWiseElementsUseBytes(subinfo); }); env = newProxy->Allocate(nobjects,true); if (!nobjects && (vers>=7)) { // Do nothing but in version 6 of TStreamerInfo and below, @@ -1303,6 +1304,9 @@ Int_t TStreamerInfo::ReadBuffer(TBuffer &b, const T &arr, TVirtualCollectionProxy::TPushPop helper( newProxy, obj ); Int_t nobjects; b >> nobjects; + nobjects = ROOT::Internal::CheckCollectionSize( + b, nobjects, "TStreamerInfo::ReadBuffer", cle->GetName(), + [&] { return ROOT::Internal::MemberWiseElementsUseBytes(subinfo); }); void* env = newProxy->Allocate(nobjects,true); if (!nobjects && (vers>=7)) { // Do nothing but in version 6 of TStreamerInfo and below, diff --git a/io/io/test/CMakeLists.txt b/io/io/test/CMakeLists.txt index ebb379cf2955d..49c7dfe265fb1 100644 --- a/io/io/test/CMakeLists.txt +++ b/io/io/test/CMakeLists.txt @@ -7,6 +7,7 @@ ROOT_ADD_GTEST(RRawFile RRawFile.cxx LIBRARIES RIO) ROOT_ADD_GTEST(TFile TFileTests.cxx LIBRARIES RIO) ROOT_ADD_GTEST(TBufferFile TBufferFileTests.cxx LIBRARIES RIO) +ROOT_ADD_GTEST(CorruptObjectCount CorruptObjectCount.cxx LIBRARIES RIO) ROOT_ADD_GTEST(TBufferMerger TBufferMerger.cxx LIBRARIES RIO Imt Tree) ROOT_ADD_GTEST(TBufferJSON TBufferJSONTests.cxx LIBRARIES RIO) ROOT_ADD_GTEST(TFileMerger TFileMergerTests.cxx LIBRARIES RIO Tree Hist) diff --git a/io/io/test/CorruptObjectCount.cxx b/io/io/test/CorruptObjectCount.cxx new file mode 100644 index 0000000000000..f6b5e57e57857 --- /dev/null +++ b/io/io/test/CorruptObjectCount.cxx @@ -0,0 +1,214 @@ +#include "gtest/gtest.h" + +#include "ROOT/TestSupport.hxx" + +#include "TArrayC.h" +#include "TArrayD.h" +#include "TArrayF.h" +#include "TArrayI.h" +#include "TArrayL.h" +#include "TArrayL64.h" +#include "TArrayS.h" +#include "TBufferFile.h" +#include "TClass.h" +#include "TClonesArray.h" +#include "TInterpreter.h" +#include "TNamed.h" +#include "TObjArray.h" +#include "TRefArray.h" + +#include +#include +#include +#include +#include + +// A number of objects read from corrupt data must not be used to allocate memory. + +namespace { + +constexpr int kN = 0x123; + +/// Position of the big-endian Int_t equal to `expected` in `data`, or data.end(). Unless `firstOccurrence`, it must +/// occur exactly once. +std::vector::iterator FindInt(std::vector &data, unsigned int expected, bool firstOccurrence) +{ + const char pattern[4] = {char(expected >> 24), char(expected >> 16), char(expected >> 8), char(expected)}; + const auto it = std::search(data.begin(), data.end(), pattern, pattern + 4); + if (it == data.end() || (!firstOccurrence && std::search(it + 1, data.end(), pattern, pattern + 4) != data.end())) { + ADD_FAILURE() << "expected exactly one occurrence of " << expected << " in the serialized data"; + return data.end(); + } + return it; +} + +/// Overwrite the big-endian Int_t at `it` with `value`. +void SetInt(std::vector::iterator it, unsigned int value) +{ + for (int i = 0; i < 4; ++i) + *(it + i) = char(value >> (8 * (3 - i))); +} + +template +std::vector Write(T &obj) +{ + TBufferFile buf(TBuffer::kWrite); + buf.WriteObject(&obj); + return std::vector(buf.Buffer(), buf.Buffer() + buf.Length()); +} + +template +std::unique_ptr Read(std::vector &data) +{ + TBufferFile buf(TBuffer::kRead, data.size(), data.data(), /*adopt=*/false); + return std::unique_ptr(static_cast(buf.ReadObject(T::Class()))); +} + +template +void CheckOversized(T &array, bool firstOccurrence = false) +{ + auto data = Write(array); + const auto it = FindInt(data, kN, firstOccurrence); + ASSERT_NE(it, data.end()); + // The smallest number of objects that cannot fit in the rest of the buffer. Without the check, reading them runs + // past the end of the buffer rather than allocating gigabytes. + SetInt(it, data.end() - (it + 4) + 1); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, std::string(T::Class_Name()) + "::Streamer", "bytes are left in the buffer", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto read = Read(data); + ASSERT_NE(read, nullptr); + EXPECT_EQ(read->GetEntriesFast(), 0); +} + +template +void CheckNegative(T &array, bool firstOccurrence = false) +{ + auto data = Write(array); + const auto it = FindInt(data, kN, firstOccurrence); + ASSERT_NE(it, data.end()); + SetInt(it, std::numeric_limits::min()); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, std::string(T::Class_Name()) + "::Streamer", "negative number of elements", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto read = Read(data); + ASSERT_NE(read, nullptr); + EXPECT_EQ(read->GetEntriesFast(), 0); +} + +template +void CheckArray(const char *location) +{ + for (bool negative : {true, false}) { + SCOPED_TRACE(negative ? "negative" : "oversized"); + T array(kN); + TBufferFile wbuf(TBuffer::kWrite); + array.Streamer(wbuf); + std::vector data(wbuf.Buffer(), wbuf.Buffer() + wbuf.Length()); + const auto it = FindInt(data, kN, /*firstOccurrence=*/false); + ASSERT_NE(it, data.end()); + SetInt(it, negative ? std::numeric_limits::min() : data.end() - (it + 4) + 1); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, location, negative ? "negative number of elements" : "bytes are left in the buffer", + /*matchFullMessage=*/false); + T read; + TBufferFile rbuf(TBuffer::kRead, data.size(), data.data(), /*adopt=*/false); + read.Streamer(rbuf); + EXPECT_EQ(read.GetSize(), 0); + } +} + +} // namespace + +TEST(CorruptObjectCount, TObjArray) +{ + TObjArray array; + array.SetOwner(); + for (int i = 0; i < kN; ++i) + array.Add(new TNamed("a", "")); + CheckOversized(array); + CheckNegative(array); +} + +TEST(CorruptObjectCount, TRefArray) +{ + std::vector objects(kN); + TRefArray array; + for (auto &obj : objects) + array.Add(&obj); + // The unique IDs of the objects come after their number and may include it. + CheckOversized(array, /*firstOccurrence=*/true); + CheckNegative(array, /*firstOccurrence=*/true); +} + +TEST(CorruptObjectCount, TClonesArray) +{ + for (bool bypass : {true, false}) { + SCOPED_TRACE(bypass ? "bypass streamer" : "object-wise"); + TClonesArray array("TNamed"); + array.BypassStreamer(bypass); + for (int i = 0; i < kN; ++i) + new (array[i]) TNamed("a", ""); + CheckOversized(array); + } +} + +// TClonesArray takes the absolute value of negative numbers of objects, which is undefined for INT_MIN. +TEST(CorruptObjectCount, TClonesArrayMinInt) +{ + TClonesArray array("TNamed"); + for (int i = 0; i < kN; ++i) + new (array[i]) TNamed("a", ""); + auto data = Write(array); + const auto it = FindInt(data, kN, /*firstOccurrence=*/false); + ASSERT_NE(it, data.end()); + SetInt(it, std::numeric_limits::min()); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TClonesArray::Streamer", "negative number of elements", /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto read = Read(data); + ASSERT_NE(read, nullptr); + EXPECT_EQ(read->GetEntriesFast(), 0); +} + +// Valid TClonesArrays whose objects occupy no space on file must still be read. +TEST(CorruptObjectCount, TClonesArrayZeroByteObjects) +{ + // The class can only be declared once, e.g. with --gtest_repeat. + static const bool declared = gInterpreter->Declare(R"CODE( + struct CorruptEmptyObject : TObject { + ClassDefOverride(CorruptEmptyObject, 1); + }; + )CODE"); + ASSERT_TRUE(declared); + TClass *cl = TClass::GetClass("CorruptEmptyObject"); + ASSERT_NE(cl, nullptr); + cl->IgnoreTObjectStreamer(); + + constexpr int kMany = 1000; + TClonesArray array("CorruptEmptyObject"); + array.ExpandCreate(kMany); + auto data = Write(array); + ASSERT_LT(data.size(), kMany); + + auto read = Read(data); + ASSERT_NE(read, nullptr); + EXPECT_EQ(read->GetEntriesFast(), kMany); +} + +TEST(CorruptObjectCount, TArray) +{ + CheckArray("TArrayC::Streamer"); + CheckArray("TArrayS::Streamer"); + CheckArray("TArrayI::Streamer"); + CheckArray("TArrayL::Streamer"); + CheckArray("TArrayL64::Streamer"); + CheckArray("TArrayF::Streamer"); + CheckArray("TArrayD::Streamer"); +} diff --git a/io/io/test/TBufferFileTests.cxx b/io/io/test/TBufferFileTests.cxx index 891f4fcf0d06d..0908ca560cd2c 100644 --- a/io/io/test/TBufferFileTests.cxx +++ b/io/io/test/TBufferFileTests.cxx @@ -1,10 +1,23 @@ #include "gtest/gtest.h" +#include "ROOT/TestSupport.hxx" + #include "TBufferFile.h" #include "TMemFile.h" #include "TClass.h" +#include "TClassStreamer.h" +#include "TClonesArray.h" +#include "TContainerConverters.h" #include "TInterpreter.h" #include "TKey.h" +#include "TMemberStreamer.h" +#include "TNamed.h" +#include "TString.h" +#include "TVirtualCollectionProxy.h" +#include +#include +#include +#include #include #include @@ -49,3 +62,605 @@ TEST(TBufferFile, ForeignZeroVersionClass) EXPECT_NE(f.Get("s1"), nullptr); EXPECT_EQ(f.GetKey("s0")->GetObjlen(), 10); } + +// A collection size read from corrupt data must not be used to allocate memory. +#define CORRUPTINNER \ + struct CorruptInner { \ + int fA = 7; \ + char fPad[1024] = {}; \ + }; +#define CORRUPTOUTER \ + struct CorruptOuter { \ + std::vector fObjs; \ + std::vector fInts; \ + }; +// Collections inside the elements of a member-wise collection are read by TStreamerInfo::ReadBuffer. +#define CORRUPTNESTED \ + struct CorruptNested { \ + std::vector fOuters; \ + }; +CORRUPTINNER +CORRUPTOUTER +CORRUPTNESTED + +namespace { + +// The element count of RawEvent::m_banks found in a corrupt LHCb file; negative as an Int_t. +constexpr unsigned int kCorruptSize = 0xa2c35db6; + +/// Position of the big-endian Int_t equal to `expected` in `data`, which must occur exactly once. +/// Returns data.size() if it does not. +std::size_t FindInt(const std::vector &data, unsigned int expected) +{ + const char pattern[4] = {char(expected >> 24), char(expected >> 16), char(expected >> 8), char(expected)}; + const auto it = std::search(data.begin(), data.end(), pattern, pattern + 4); + if (it == data.end() || std::search(it + 1, data.end(), pattern, pattern + 4) != data.end()) { + ADD_FAILURE() << "expected exactly one occurrence of " << expected << " in the serialized data"; + return data.size(); + } + return it - data.begin(); +} + +/// Overwrite the big-endian Int_t equal to `expected` in `data` with `value`. +void CorruptInt(std::vector &data, unsigned int expected, unsigned int value) +{ + const auto pos = FindInt(data, expected); + if (pos == data.size()) + return; + for (int i = 0; i < 4; ++i) + data[pos + i] = char(value >> (8 * (3 - i))); +} + +/// Bytes left in `data` after the big-endian Int_t equal to `expected`. +unsigned int BytesAfterInt(const std::vector &data, unsigned int expected) +{ + return data.size() - std::min(data.size(), FindInt(data, expected) + 4); +} + +std::vector Write(const void *obj, TClass *cl) +{ + TBufferFile buf(TBuffer::kWrite); + buf.WriteObjectAny(obj, cl); + return std::vector(buf.Buffer(), buf.Buffer() + buf.Length()); +} + +void *Read(std::vector &data, TClass *cl) +{ + TBufferFile buf(TBuffer::kRead, data.size(), data.data(), /*adopt=*/false); + return buf.ReadObjectAny(cl); +} + +template +std::vector Write(const T &obj) +{ + return Write(&obj, TClass::GetClass()); +} + +template +std::unique_ptr Read(std::vector &data) +{ + return std::unique_ptr(static_cast(Read(data, TClass::GetClass()))); +} + +/// Number of elements of the collection of type `cl` at `obj`. +UInt_t CollectionSize(TClass *cl, void *obj) +{ + TVirtualCollectionProxy::TPushPop helper(cl->GetCollectionProxy(), obj); + return cl->GetCollectionProxy()->Size(); +} + +/// Resize the collection of type `cl` at `obj` to `n` elements. +void ResizeCollection(TClass *cl, void *obj, UInt_t n) +{ + TVirtualCollectionProxy::TPushPop helper(cl->GetCollectionProxy(), obj); + cl->GetCollectionProxy()->Commit(cl->GetCollectionProxy()->Allocate(n, true)); +} + +constexpr int kTextLength = 0x135; + +/// Write a CorruptEvoA with a text of kTextLength characters, `nFloats` floats and twice `nElems` elements that occupy +/// no space to a file, and rename its classes to CorruptEvoB and CorruptEvoBElem, so that reading it back goes through +/// schema evolution. +std::vector WriteEvolvedFile(int nFloats, int nElems) +{ + TClass *cl = TClass::GetClass("CorruptEvoA"); + void *obj = cl->New(); + std::string text(kTextLength, 'x'); + auto *textPtr = reinterpret_cast(static_cast(obj) + cl->GetDataMemberOffset("fText")); + *textPtr = text.data(); + auto *floats = reinterpret_cast *>(static_cast(obj) + cl->GetDataMemberOffset("fFloats")); + floats->assign(nFloats, 1.5f); + ResizeCollection(TClass::GetClass("vector"), + static_cast(obj) + cl->GetDataMemberOffset("fElems"), nElems); + ResizeCollection(TClass::GetClass("vector"), + static_cast(obj) + cl->GetDataMemberOffset("fRenamed"), nElems); + std::vector data; + { + TMemFile file("evolution.root", "RECREATE", "", /*compress=*/0); + file.WriteObjectAny(obj, cl, "obj"); + file.Write(); + data.resize(file.GetSize()); + file.CopyTo(data.data(), data.size()); + } + *textPtr = nullptr; + cl->Destructor(obj); + const std::string from = "CorruptEvoA", to = "CorruptEvoB"; + for (auto it = data.begin(); (it = std::search(it, data.end(), from.begin(), from.end())) != data.end();) + it = std::copy(to.begin(), to.end(), it); + return data; +} + +void *ReadEvolvedFile(std::vector &data) +{ + TMemFile file("evolution.root", TMemFile::ZeroCopyView_t(data.data(), data.size())); + return file.GetObjectChecked("obj", "CorruptEvoB"); +} + +class CorruptCollectionSize : public ::testing::Test { +protected: + static constexpr int kNObjs = 0x123; + static constexpr int kNInts = 0x321; + static constexpr int kNFloats = 0x2345; + + std::vector fData; + + static void SetUpTestSuite() + { + // Classes can only be declared and their streamers adopted once, e.g. with --gtest_repeat. + static bool done = false; + if (done) + return; + done = true; + gInterpreter->Declare(TO_LITERAL(CORRUPTINNER)); + gInterpreter->Declare(TO_LITERAL(CORRUPTOUTER)); + gInterpreter->Declare(TO_LITERAL(CORRUPTNESTED)); + // Elements of a vector occupy no space on file. + gInterpreter->Declare(R"CODE( + struct CorruptNoBytes {}; + struct CorruptInPlace { + CorruptNoBytes *fObj = new CorruptNoBytes; //-> + }; + struct CorruptZeroByteHolder { + std::vector fInPlace; + std::vector fPtrs; + }; + // Version 1 is written as CorruptEvoA, and read back as CorruptEvoB after renaming it in the file. + struct CorruptEvoAElem { + CorruptNoBytes *fObj = new CorruptNoBytes; //-> + virtual ~CorruptEvoAElem() = default; + ClassDef(CorruptEvoAElem, 1); + }; + // The only data member of CorruptCustomMember is streamed by a custom member streamer that writes nothing. + struct CorruptCustomMember : TObject { + TObject *fObj = nullptr; + ClassDefOverride(CorruptCustomMember, 1); + }; + // CorruptOldElem is read as CorruptNewElem through a rename rule. + struct CorruptOldElem { + CorruptNoBytes *fObj = new CorruptNoBytes; //-> + virtual ~CorruptOldElem() = default; + ClassDef(CorruptOldElem, 1); + }; + struct CorruptNewElem { + CorruptNoBytes *fObj = new CorruptNoBytes; //-> + int fNew = 3; + virtual ~CorruptNewElem() = default; + ClassDef(CorruptNewElem, 1); + }; + struct CorruptEvoA { + std::vector fFloats; + std::vector fElems; + std::vector fRenamed; + char *fText = nullptr; // Removed in version 2 + virtual ~CorruptEvoA() = default; + ClassDef(CorruptEvoA, 1); + }; + struct CorruptEvoBElem { + CorruptNoBytes *fObj = new CorruptNoBytes; //-> + int fNew = 3; + virtual ~CorruptEvoBElem() = default; + ClassDef(CorruptEvoBElem, 2); + }; + struct CorruptEvoB { + std::vector fFloats; + std::vector fElems; + std::vector fRenamed; + virtual ~CorruptEvoB() = default; + ClassDef(CorruptEvoB, 2); + }; + )CODE"); + TClass::GetClass("CorruptNoBytes")->AdoptStreamer(new TClassStreamer([](TBuffer &, void *) {})); + TClass *customMember = TClass::GetClass("CorruptCustomMember"); + customMember->BuildRealData(); + customMember->AdoptMemberStreamer("*fObj", new TMemberStreamer([](TBuffer &, void *, Int_t) {})); + customMember->IgnoreTObjectStreamer(); + TClass::AddRule("CorruptNewElem version=[1-] sourceClass=CorruptOldElem targetClass=CorruptNewElem"); + } + + void SetUp() override + { + CorruptOuter outer; + outer.fObjs.resize(kNObjs); + outer.fInts.assign(kNInts, 1); + fData = Write(outer); + } +}; + +} // namespace + +TEST_F(CorruptCollectionSize, MemberWise) +{ + CorruptInt(fData, kNObjs, kCorruptSize); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TStreamerInfoActions::ReadSTLMemberWiseSameClass", "negative number of elements", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto outer = Read(fData); + ASSERT_NE(outer, nullptr); + EXPECT_TRUE(outer->fObjs.empty()); + EXPECT_EQ(outer->fInts.size(), kNInts); +} + +TEST_F(CorruptCollectionSize, BasicType) +{ + CorruptInt(fData, kNInts, kCorruptSize); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TStreamerInfoActions::ReadCollectionBasicType", "negative number of elements", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto outer = Read(fData); + ASSERT_NE(outer, nullptr); + EXPECT_EQ(outer->fObjs.size(), kNObjs); + EXPECT_TRUE(outer->fInts.empty()); +} + +// Collections without a dictionary are read through TEmulatedCollectionProxy and TEmulatedMapProxy. +TEST_F(CorruptCollectionSize, Emulated) +{ + for (auto [name, location] : {std::make_pair("vector", "TEmulatedCollectionProxy::ReadBuffer"), + std::make_pair("map", "TEmulatedMapProxy::ReadBuffer")}) { + SCOPED_TRACE(name); + TClass *cl = TClass::GetClass(name); + ASSERT_NE(cl, nullptr); + ASSERT_TRUE(cl->GetCollectionProxy()->GetProperties() & TVirtualCollectionProxy::kIsEmulated); + void *obj = cl->New(); + { + // Emulated maps are stored like vectors of pairs, so the duplicate keys do not matter. + TVirtualCollectionProxy::TPushPop helper(cl->GetCollectionProxy(), obj); + cl->GetCollectionProxy()->Commit(cl->GetCollectionProxy()->Allocate(kNObjs, true)); + } + ASSERT_EQ(CollectionSize(cl, obj), kNObjs); + auto data = Write(obj, cl); + cl->Destructor(obj); + CorruptInt(data, kNObjs, kCorruptSize); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, location, "negative number of elements", /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + void *read = Read(data, cl); + ASSERT_NE(read, nullptr); + EXPECT_EQ(CollectionSize(cl, read), 0u); + cl->Destructor(read); + } +} + +TEST_F(CorruptCollectionSize, Nested) +{ + CorruptNested nested; + nested.fOuters.resize(1); + nested.fOuters[0].fObjs.resize(kNObjs); + auto data = Write(nested); + CorruptInt(data, kNObjs, kCorruptSize); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TStreamerInfo::ReadBuffer", "negative number of elements", /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto read = Read(data); + ASSERT_NE(read, nullptr); + ASSERT_EQ(read->fOuters.size(), 1u); + EXPECT_TRUE(read->fOuters[0].fObjs.empty()); +} + +// A TClonesArray on file is read into an STL collection by TConvertClonesArrayToProxy, which takes the +// absolute value of negative sizes. +TEST_F(CorruptCollectionSize, ClonesArrayToProxy) +{ + TClonesArray clones("TNamed", kNObjs); + for (int i = 0; i < kNObjs; ++i) + clones.ConstructedAt(i); + TBufferFile wbuf(TBuffer::kWrite); + clones.Streamer(wbuf); + std::vector data(wbuf.Buffer(), wbuf.Buffer() + wbuf.Length()); + CorruptInt(data, kNObjs, std::numeric_limits::min()); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TConvertClonesArrayToProxy::operator()", "negative number of elements", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + TClass *cl = TClass::GetClass("vector"); + ASSERT_NE(cl, nullptr); + TConvertClonesArrayToProxy convert(cl->GetCollectionProxy(), /*isPointer=*/false, /*isPrealloc=*/false); + void *read = cl->New(); + TBufferFile rbuf(TBuffer::kRead, data.size(), data.data(), /*adopt=*/false); + convert(rbuf, read); + EXPECT_EQ(CollectionSize(cl, read), 0u); + cl->Destructor(read); +} + +// A size larger than the bytes left in the buffer cannot be valid if each element occupies at least one byte. +TEST_F(CorruptCollectionSize, MemberWiseOversized) +{ + CorruptInt(fData, kNObjs, BytesAfterInt(fData, kNObjs) + 1); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TStreamerInfoActions::ReadSTLMemberWiseSameClass", "bytes are left in the buffer", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto outer = Read(fData); + ASSERT_NE(outer, nullptr); + EXPECT_TRUE(outer->fObjs.empty()); + EXPECT_EQ(outer->fInts.size(), kNInts); +} + +TEST_F(CorruptCollectionSize, NestedOversized) +{ + CorruptNested nested; + nested.fOuters.resize(1); + nested.fOuters[0].fObjs.resize(kNObjs); + auto data = Write(nested); + CorruptInt(data, kNObjs, BytesAfterInt(data, kNObjs) + 1); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TStreamerInfo::ReadBuffer", "bytes are left in the buffer", /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto read = Read(data); + ASSERT_NE(read, nullptr); + ASSERT_EQ(read->fOuters.size(), 1u); + EXPECT_TRUE(read->fOuters[0].fObjs.empty()); +} + +TEST_F(CorruptCollectionSize, ClonesArrayToProxyOversized) +{ + TClass *cl = TClass::GetClass("vector"); + ASSERT_NE(cl, nullptr); + for (bool bypass : {true, false}) { + SCOPED_TRACE(bypass ? "member-wise" : "object-wise"); + TClonesArray clones("TNamed", kNObjs); + clones.BypassStreamer(bypass); + for (int i = 0; i < kNObjs; ++i) + clones.ConstructedAt(i); + TBufferFile wbuf(TBuffer::kWrite); + clones.Streamer(wbuf); + std::vector data(wbuf.Buffer(), wbuf.Buffer() + wbuf.Length()); + CorruptInt(data, kNObjs, BytesAfterInt(data, kNObjs) + 1); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TConvertClonesArrayToProxy::operator()", "bytes are left in the buffer", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + TConvertClonesArrayToProxy convert(cl->GetCollectionProxy(), /*isPointer=*/false, /*isPrealloc=*/false); + void *read = cl->New(); + TBufferFile rbuf(TBuffer::kRead, data.size(), data.data(), /*adopt=*/false); + convert(rbuf, read); + EXPECT_EQ(CollectionSize(cl, read), 0u); + cl->Destructor(read); + } +} + +TEST_F(CorruptCollectionSize, BasicTypeOversized) +{ + CorruptInt(fData, kNInts, BytesAfterInt(fData, kNInts) + 1); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TStreamerInfoActions::ReadCollectionBasicType", "bytes are left in the buffer", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto outer = Read(fData); + ASSERT_NE(outer, nullptr); + EXPECT_EQ(outer->fObjs.size(), kNObjs); + EXPECT_TRUE(outer->fInts.empty()); +} + +// A size equal to the bytes left in the buffer is not rejected. +TEST_F(CorruptCollectionSize, BasicTypeBoundary) +{ + const auto size = BytesAfterInt(fData, kNInts); + CorruptInt(fData, kNInts, size); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto outer = Read(fData); + ASSERT_NE(outer, nullptr); + EXPECT_EQ(outer->fInts.size(), size); +} + +TEST_F(CorruptCollectionSize, ObjectWiseOversized) +{ + std::vector strings(kNObjs, "a"); + auto data = Write(strings); + CorruptInt(data, kNObjs, BytesAfterInt(data, kNObjs) + 1); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TGenCollectionStreamer::ReadBufferGeneric", "bytes are left in the buffer", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto read = Read>(data); + ASSERT_NE(read, nullptr); + EXPECT_TRUE(read->empty()); +} + +TEST_F(CorruptCollectionSize, EmulatedOversized) +{ + TClass *cl = TClass::GetClass("map"); + ASSERT_NE(cl, nullptr); + void *obj = cl->New(); + { + TVirtualCollectionProxy::TPushPop helper(cl->GetCollectionProxy(), obj); + cl->GetCollectionProxy()->Commit(cl->GetCollectionProxy()->Allocate(kNObjs, true)); + } + auto data = Write(obj, cl); + cl->Destructor(obj); + CorruptInt(data, kNObjs, BytesAfterInt(data, kNObjs) + 1); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TEmulatedMapProxy::ReadBuffer", "bytes are left in the buffer", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + void *read = Read(data, cl); + ASSERT_NE(read, nullptr); + EXPECT_EQ(CollectionSize(cl, read), 0u); + cl->Destructor(read); +} + +// Valid collections whose elements occupy no space on file must still be read. +TEST_F(CorruptCollectionSize, ZeroByteElements) +{ + constexpr int kN = 1000; + TClass *cl = TClass::GetClass("CorruptZeroByteHolder"); + ASSERT_NE(cl, nullptr); + TClass *inPlace = TClass::GetClass("vector"); + TClass *ptrs = TClass::GetClass("vector"); + void *obj = cl->New(); + ResizeCollection(inPlace, static_cast(obj) + cl->GetDataMemberOffset("fInPlace"), kN); + ResizeCollection(ptrs, static_cast(obj) + cl->GetDataMemberOffset("fPtrs"), kN); + auto data = Write(obj, cl); + cl->Destructor(obj); + ASSERT_LT(data.size(), kN); + void *read = Read(data, cl); + ASSERT_NE(read, nullptr); + EXPECT_EQ(CollectionSize(inPlace, static_cast(read) + cl->GetDataMemberOffset("fInPlace")), kN); + EXPECT_EQ(CollectionSize(ptrs, static_cast(read) + cl->GetDataMemberOffset("fPtrs")), kN); + cl->Destructor(read); +} + +// The same holds for a TClonesArray read into a vector, here with objects whose only member has a custom streamer. +TEST_F(CorruptCollectionSize, ZeroByteClonesArrayToProxy) +{ + constexpr int kN = 1000; + TClonesArray clones("CorruptCustomMember", kN); + clones.ExpandCreate(kN); + TBufferFile wbuf(TBuffer::kWrite); + clones.Streamer(wbuf); + std::vector data(wbuf.Buffer(), wbuf.Buffer() + wbuf.Length()); + ASSERT_LT(data.size(), kN); + + TClass *cl = TClass::GetClass("vector"); + ASSERT_NE(cl, nullptr); + TConvertClonesArrayToProxy convert(cl->GetCollectionProxy(), /*isPointer=*/false, /*isPrealloc=*/false); + void *read = cl->New(); + TBufferFile rbuf(TBuffer::kRead, data.size(), data.data(), /*adopt=*/false); + convert(rbuf, read); + EXPECT_EQ(CollectionSize(cl, read), kN); + cl->Destructor(read); +} + +// The same holds when reading through schema evolution, which uses the layout on file: here from vector +// into vector, and elements that occupy no space on file but gained a data member, of the same class or of a +// class renamed by a rule. +TEST_F(CorruptCollectionSize, SchemaEvolution) +{ + constexpr int kNElems = 1000; + auto data = WriteEvolvedFile(kNFloats, kNElems); + TClass *cl = TClass::GetClass("CorruptEvoB"); + ROOT::TestSupport::CheckDiagsRAII diags; + auto *read = static_cast(ReadEvolvedFile(data)); + ASSERT_NE(read, nullptr); + auto *doubles = reinterpret_cast *>(read + cl->GetDataMemberOffset("fFloats")); + ASSERT_EQ(doubles->size(), kNFloats); + EXPECT_EQ(doubles->front(), 1.5); + EXPECT_EQ(CollectionSize(TClass::GetClass("vector"), read + cl->GetDataMemberOffset("fElems")), + kNElems); + EXPECT_EQ(CollectionSize(TClass::GetClass("vector"), read + cl->GetDataMemberOffset("fRenamed")), + kNElems); + cl->Destructor(read); +} + +TEST_F(CorruptCollectionSize, SchemaEvolutionOversized) +{ + auto data = WriteEvolvedFile(kNFloats, 1); + // The rest of the file may contain the same 4 bytes, so look for the size followed by the first value, 1.5f. + const char pattern[8] = { + char(kNFloats >> 24), char(kNFloats >> 16), char(kNFloats >> 8), char(kNFloats), 0x3f, char(0xc0), 0, 0}; + const auto pos = std::search(data.begin(), data.end(), pattern, pattern + 8); + ASSERT_NE(pos, data.end()); + // One more than the bytes left after the size. + const unsigned int size = data.end() - (pos + 4) + 1; + for (int i = 0; i < 4; ++i) + pos[i] = char(size >> (8 * (3 - i))); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TStreamerInfoActions::ConvertCollectionBasicType", "bytes are left in the buffer", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + TClass *cl = TClass::GetClass("CorruptEvoB"); + auto *read = static_cast(ReadEvolvedFile(data)); + ASSERT_NE(read, nullptr); + EXPECT_TRUE(reinterpret_cast *>(read + cl->GetDataMemberOffset("fFloats"))->empty()); + cl->Destructor(read); +} + +// Strings longer than 254 characters store their length as an Int_t. +TEST_F(CorruptCollectionSize, String) +{ + constexpr unsigned int kLength = 0x1234; + const auto checkCorruptLength = [](auto str, const char *location, bool negative) { + auto data = Write(str); + CorruptInt(data, kLength, negative ? kCorruptSize : BytesAfterInt(data, kLength) + 1); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, location, negative ? "negative length" : "bytes are left in the buffer", + /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + auto read = Read(data); + ASSERT_NE(read, nullptr); + EXPECT_TRUE(*read == decltype(str)()); + }; + for (bool negative : {true, false}) { + SCOPED_TRACE(negative ? "negative" : "oversized"); + checkCorruptLength(std::string(kLength, 'a'), "TBufferFile::ReadStdString", negative); + checkCorruptLength(TString('a', kLength), "TBufferFile::ReadTString", negative); + } +} + +TEST_F(CorruptCollectionSize, CharStar) +{ + constexpr unsigned int kLength = 0x1234; + const std::string str(kLength, 'a'); + TBufferFile wbuf(TBuffer::kWrite); + wbuf.WriteCharStar(const_cast(str.c_str())); + std::vector data(wbuf.Buffer(), wbuf.Buffer() + wbuf.Length()); + CorruptInt(data, kLength, BytesAfterInt(data, kLength) + 1); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TBufferFile::ReadCharStar", "bytes are left in the buffer", /*matchFullMessage=*/false); + TBufferFile rbuf(TBuffer::kRead, data.size(), data.data(), /*adopt=*/false); + char *read = nullptr; + rbuf.ReadCharStar(read); + EXPECT_EQ(read, nullptr); + delete[] read; +} + +// A char* data member that was removed is skipped with ReadCharStar. +TEST_F(CorruptCollectionSize, SkippedCharStar) +{ + auto data = WriteEvolvedFile(kNFloats, 1); + // The rest of the file may contain the same 4 bytes, so look for the length followed by the first characters. + const char pattern[8] = { + char(kTextLength >> 24), char(kTextLength >> 16), char(kTextLength >> 8), char(kTextLength), 'x', 'x', 'x', 'x'}; + const auto pos = std::search(data.begin(), data.end(), pattern, pattern + 8); + ASSERT_NE(pos, data.end()); + // One more than the bytes left after the length. + const unsigned int size = data.end() - (pos + 4) + 1; + for (int i = 0; i < 4; ++i) + pos[i] = char(size >> (8 * (3 - i))); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "TBufferFile::ReadCharStar", "bytes are left in the buffer", /*matchFullMessage=*/false); + diags.optionalDiag(kError, "TBufferFile::CheckByteCount", "read too few bytes", /*matchFullMessage=*/false); + TClass *cl = TClass::GetClass("CorruptEvoB"); + auto *read = static_cast(ReadEvolvedFile(data)); + ASSERT_NE(read, nullptr); + cl->Destructor(read); +}