From 15a9df04a793d432b00a950e8bce8aaa5848ec49 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 17 Dec 2025 22:04:13 +0100 Subject: [PATCH 1/8] [core] improve signature of low-level (un)zip functions Make the source pointer const. --- core/lz4/inc/ZipLZ4.h | 4 ++-- core/lz4/src/ZipLZ4.cxx | 6 ++--- core/lzma/inc/ZipLZMA.h | 4 ++-- core/lzma/src/ZipLZMA.c | 4 ++-- core/zip/inc/RZip.h | 10 ++++----- core/zip/src/Bits.c | 2 +- core/zip/src/Bits.h | 7 +++--- core/zip/src/RZip.cxx | 34 +++++++++++++++-------------- core/zstd/inc/ZipZSTD.h | 4 ++-- core/zstd/src/ZipZSTD.cxx | 4 ++-- io/io/src/TBufferJSON.cxx | 2 +- tree/ntuple/inc/ROOT/RNTupleZip.hxx | 4 ++-- tree/tree/src/TTreeCacheUnzip.cxx | 6 ++--- 13 files changed, 46 insertions(+), 45 deletions(-) diff --git a/core/lz4/inc/ZipLZ4.h b/core/lz4/inc/ZipLZ4.h index 1ff5dda7d2588..abe5083066e3b 100644 --- a/core/lz4/inc/ZipLZ4.h +++ b/core/lz4/inc/ZipLZ4.h @@ -16,8 +16,8 @@ #ifdef __cplusplus extern "C" { #endif -void R__zipLZ4(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, int *irep); -void R__unzipLZ4(int *srcsize, unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep); +void R__zipLZ4(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgt, int *irep); +void R__unzipLZ4(int *srcsize, const unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep); #ifdef __cplusplus } #endif diff --git a/core/lz4/src/ZipLZ4.cxx b/core/lz4/src/ZipLZ4.cxx index 6427953f94fcf..6fdafb851f5ba 100644 --- a/core/lz4/src/ZipLZ4.cxx +++ b/core/lz4/src/ZipLZ4.cxx @@ -30,7 +30,7 @@ static const int kChecksumOffset = 2 + 1 + 3 + 3; static const int kChecksumSize = sizeof(XXH64_canonical_t); static const int kHeaderSize = kChecksumOffset + kChecksumSize; -void R__zipLZ4(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, int *irep) +void R__zipLZ4(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgt, int *irep) { int LZ4_version = LZ4_versionNumber(); uint64_t out_size; /* compressed size */ @@ -84,7 +84,7 @@ void R__zipLZ4(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, in *irep = (int)returnStatus + kHeaderSize; } -void R__unzipLZ4(int *srcsize, unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep) +void R__unzipLZ4(int *srcsize, const unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep) { // NOTE: We don't check that srcsize / tgtsize is reasonable or within the ROOT-imposed limits. // This is assumed to be handled by the upper layers. @@ -111,7 +111,7 @@ void R__unzipLZ4(int *srcsize, unsigned char *src, int *tgtsize, unsigned char * // extra function call costs? NOTE that ROOT limits the buffer size to 16MB. XXH64_hash_t checksumResult = XXH64(src + kHeaderSize, inputBufferSize, 0); XXH64_hash_t checksumFromFile = - XXH64_hashFromCanonical(reinterpret_cast(src + kChecksumOffset)); + XXH64_hashFromCanonical(reinterpret_cast(src + kChecksumOffset)); if (R__unlikely(checksumFromFile != checksumResult)) { fprintf( diff --git a/core/lzma/inc/ZipLZMA.h b/core/lzma/inc/ZipLZMA.h index 90bd0c60dd4b4..35897965bc6a3 100644 --- a/core/lzma/inc/ZipLZMA.h +++ b/core/lzma/inc/ZipLZMA.h @@ -16,9 +16,9 @@ extern "C" { #endif -void R__zipLZMA(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, int *irep); +void R__zipLZMA(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgt, int *irep); -void R__unzipLZMA(int *srcsize, unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep); +void R__unzipLZMA(int *srcsize, const unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep); #ifdef __cplusplus } diff --git a/core/lzma/src/ZipLZMA.c b/core/lzma/src/ZipLZMA.c index eae42fc6ae243..4582c59873837 100644 --- a/core/lzma/src/ZipLZMA.c +++ b/core/lzma/src/ZipLZMA.c @@ -18,7 +18,7 @@ static const int kHeaderSize = 9; -void R__zipLZMA(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, int *irep) +void R__zipLZMA(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgt, int *irep) { uint64_t out_size; /* compressed size */ unsigned in_size = (unsigned) (*srcsize); @@ -99,7 +99,7 @@ void R__zipLZMA(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, i *irep = (int)stream.total_out + kHeaderSize; } -void R__unzipLZMA(int *srcsize, unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep) +void R__unzipLZMA(int *srcsize, const unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep) { lzma_stream stream = LZMA_STREAM_INIT; lzma_ret returnStatus; diff --git a/core/zip/inc/RZip.h b/core/zip/inc/RZip.h index 404587324cf2c..3da41c0914c0e 100644 --- a/core/zip/inc/RZip.h +++ b/core/zip/inc/RZip.h @@ -19,18 +19,18 @@ extern "C" unsigned long R__crc32(unsigned long crc, const unsigned char *buf, unsigned int len); -extern "C" unsigned long R__memcompress(char *tgt, unsigned long tgtsize, char *src, unsigned long srcsize); +extern "C" unsigned long R__memcompress(char *tgt, unsigned long tgtsize, const char *src, unsigned long srcsize); -extern "C" void R__zipMultipleAlgorithm(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, int *irep, +extern "C" void R__zipMultipleAlgorithm(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgt, int *irep, ROOT::RCompressionSetting::EAlgorithm::EValues algorithm); -extern "C" ROOT::RCompressionSetting::EAlgorithm::EValues R__getCompressionAlgorithm(const unsigned char *buf, +extern "C" ROOT::RCompressionSetting::EAlgorithm::EValues R__getCompressionAlgorithm(const unsigned char *buf, size_t bufsize); -extern "C" void R__unzip(int *srcsize, unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep); +extern "C" void R__unzip(int *srcsize, const unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep); -extern "C" int R__unzip_header(int *srcsize, unsigned char *src, int *tgtsize); +extern "C" int R__unzip_header(int *srcsize, const unsigned char *src, int *tgtsize); enum { kMAXZIPBUF = 0xffffff }; // 16 MB diff --git a/core/zip/src/Bits.c b/core/zip/src/Bits.c index e822cf9c0afd9..85ef1712f1178 100644 --- a/core/zip/src/Bits.c +++ b/core/zip/src/Bits.c @@ -291,7 +291,7 @@ int R__seekable() * the first six bytes (method and crc). */ -ulg R__memcompress(char *tgt, ulg tgtsize, char *src, ulg srcsize) +ulg R__memcompress(char *tgt, ulg tgtsize, const char *src, ulg srcsize) /* char *tgt, *src; target and source buffers */ /* ulg tgtsize, srcsize; target and source sizes */ { diff --git a/core/zip/src/Bits.h b/core/zip/src/Bits.h index fe3b366320207..525e3724ff9b2 100644 --- a/core/zip/src/Bits.h +++ b/core/zip/src/Bits.h @@ -55,7 +55,8 @@ struct bits_internal_state { * are always zero. */ - char *in_buf, *out_buf; + const char *in_buf; + char *out_buf; /* Current input and output buffers. in_buf is used only for in-memory * compression. */ @@ -239,13 +240,13 @@ int R__mem_read OF((bits_internal_state *state, char *b, unsigned bsize) */ /* char *tgt, *src; target and source buffers */ /* ulg tgtsize, srcsize; target and source sizes */ -ulg R__memcompress(char *tgt, ulg tgtsize, char *src, ulg srcsize); +ulg R__memcompress(char *tgt, ulg tgtsize, const char *src, ulg srcsize); /** * Decompress a deflated entry. */ -int R__Inflate(uch** ibufptr, long* ibufcnt, uch** obufptr, long* obufcnt); +int R__Inflate(const uch** ibufptr, long* ibufcnt, uch** obufptr, long* obufcnt); #ifdef __cplusplus } // extern "C" diff --git a/core/zip/src/RZip.cxx b/core/zip/src/RZip.cxx index 6e970442a50a3..ec5e0ae95e518 100644 --- a/core/zip/src/RZip.cxx +++ b/core/zip/src/RZip.cxx @@ -28,9 +28,9 @@ /** * Forward decl's */ -static void R__zipOld(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgrt, int *irep); -static void R__zipZLIB(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgrt, int *irep); -static void R__unzipZLIB(int *srcsize, unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep); +static void R__zipOld(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgrt, int *irep); +static void R__zipZLIB(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgrt, int *irep); +static void R__unzipZLIB(int *srcsize, const unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep); /* =========================================================================== R__ZipMode is used to select the compression algorithm when R__zip is called @@ -76,7 +76,8 @@ unsigned long R__crc32(unsigned long crc, const unsigned char* buf, unsigned int /* 1 = zlib */ /* 2 = lzma */ /* 3 = old */ -void R__zipMultipleAlgorithm(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, int *irep, ROOT::RCompressionSetting::EAlgorithm::EValues compressionAlgorithm) +void R__zipMultipleAlgorithm(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgt, int *irep, + ROOT::RCompressionSetting::EAlgorithm::EValues compressionAlgorithm) { *irep = 0; @@ -117,7 +118,7 @@ void R__zipMultipleAlgorithm(int cxlevel, int *srcsize, char *src, int *tgtsize, // The very old algorithm for backward compatibility // 0 for selecting with R__ZipMode in a backward compatible way // 3 for selecting in other cases -static void R__zipOld(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, int *irep) +static void R__zipOld(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgt, int *irep) { int method = Z_DEFLATED; bits_internal_state state; @@ -178,7 +179,7 @@ static void R__zipOld(int cxlevel, int *srcsize, char *src, int *tgtsize, char * /** * Compress buffer contents using the venerable zlib algorithm. */ -static void R__zipZLIB(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, int *irep) +static void R__zipZLIB(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgt, int *irep) { int err; int method = Z_DEFLATED; @@ -246,32 +247,32 @@ static void R__zipZLIB(int cxlevel, int *srcsize, char *src, int *tgtsize, char * Below are the routines for unzipping (inflating) buffers. */ -static int is_valid_header_zlib(unsigned char *src) +static int is_valid_header_zlib(const unsigned char *src) { return src[0] == 'Z' && src[1] == 'L' && src[2] == Z_DEFLATED; } -static int is_valid_header_old(unsigned char *src) +static int is_valid_header_old(const unsigned char *src) { return src[0] == 'C' && src[1] == 'S' && src[2] == Z_DEFLATED; } -static int is_valid_header_lzma(unsigned char *src) +static int is_valid_header_lzma(const unsigned char *src) { return src[0] == 'X' && src[1] == 'Z' && src[2] == 0; } -static int is_valid_header_lz4(unsigned char *src) +static int is_valid_header_lz4(const unsigned char *src) { return src[0] == 'L' && src[1] == '4'; } -static int is_valid_header_zstd(unsigned char *src) +static int is_valid_header_zstd(const unsigned char *src) { return src[0] == 'Z' && src[1] == 'S' && src[2] == '\1'; } -static int is_valid_header(unsigned char *src) +static int is_valid_header(const unsigned char *src) { return is_valid_header_zlib(src) || is_valid_header_old(src) || is_valid_header_lzma(src) || is_valid_header_lz4(src) || is_valid_header_zstd(src); @@ -296,7 +297,7 @@ ROOT::RCompressionSetting::EAlgorithm::EValues R__getCompressionAlgorithm(const return ROOT::RCompressionSetting::EAlgorithm::kUndefined; } -int R__unzip_header(int *srcsize, uch *src, int *tgtsize) +int R__unzip_header(int *srcsize, const uch *src, int *tgtsize) { // Reads header envelope, and determines target size. // Returns 0 in case of success. @@ -336,10 +337,11 @@ int R__unzip_header(int *srcsize, uch *src, int *tgtsize) ***********************************************************************/ // N.B. (Brian) - I have kept the original note out of complete awe of the // age of the original code... -void R__unzip(int *srcsize, uch *src, int *tgtsize, uch *tgt, int *irep) +void R__unzip(int *srcsize, const uch *src, int *tgtsize, uch *tgt, int *irep) { long isize; - uch *ibufptr, *obufptr; + const uch *ibufptr; + uch *obufptr; long ibufcnt, obufcnt; *irep = 0L; @@ -407,7 +409,7 @@ void R__unzip(int *srcsize, uch *src, int *tgtsize, uch *tgt, int *irep) *irep = isize; } -void R__unzipZLIB(int *srcsize, unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep) +void R__unzipZLIB(int *srcsize, const unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep) { z_stream stream; /* decompression stream */ int err = 0; diff --git a/core/zstd/inc/ZipZSTD.h b/core/zstd/inc/ZipZSTD.h index 797b315e6b638..b26925e77a1e7 100644 --- a/core/zstd/inc/ZipZSTD.h +++ b/core/zstd/inc/ZipZSTD.h @@ -15,8 +15,8 @@ #ifdef __cplusplus extern "C" { #endif -void R__zipZSTD(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, int *irep); -void R__unzipZSTD(int *srcsize, unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep); +void R__zipZSTD(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgt, int *irep); +void R__unzipZSTD(int *srcsize, const unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep); #ifdef __cplusplus } #endif diff --git a/core/zstd/src/ZipZSTD.cxx b/core/zstd/src/ZipZSTD.cxx index 05fe13514271d..a9b6f61dfd080 100644 --- a/core/zstd/src/ZipZSTD.cxx +++ b/core/zstd/src/ZipZSTD.cxx @@ -22,7 +22,7 @@ static const int kHeaderSize = 9; static const size_t errorCodeSmallBuffer = (size_t)-70; -void R__zipZSTD(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, int *irep) +void R__zipZSTD(int cxlevel, int *srcsize, const char *src, int *tgtsize, char *tgt, int *irep) { using Ctx_ptr = std::unique_ptr; Ctx_ptr fCtx{ZSTD_createCCtx(), &ZSTD_freeCCtx}; @@ -58,7 +58,7 @@ void R__zipZSTD(int cxlevel, int *srcsize, char *src, int *tgtsize, char *tgt, i tgt[8] = (inflate_size >> 16) & 0xff; } -void R__unzipZSTD(int *srcsize, unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep) +void R__unzipZSTD(int *srcsize, const unsigned char *src, int *tgtsize, unsigned char *tgt, int *irep) { using Ctx_ptr = std::unique_ptr; Ctx_ptr fCtx{ZSTD_createDCtx(), &ZSTD_freeDCtx}; diff --git a/io/io/src/TBufferJSON.cxx b/io/io/src/TBufferJSON.cxx index 23ff6091ea355..253ffd9e621d1 100644 --- a/io/io/src/TBufferJSON.cxx +++ b/io/io/src/TBufferJSON.cxx @@ -563,7 +563,7 @@ TString TBufferJSON::zipJSON(const char *json) int nout = 0; - R__zipMultipleAlgorithm(ROOT::RCompressionSetting::ELevel::kDefaultZLIB, &srcsize, (char *)json, &tgtsize, (char *) buf.data(), &nout, ROOT::RCompressionSetting::EAlgorithm::kZLIB); + R__zipMultipleAlgorithm(ROOT::RCompressionSetting::ELevel::kDefaultZLIB, &srcsize, json, &tgtsize, (char *) buf.data(), &nout, ROOT::RCompressionSetting::EAlgorithm::kZLIB); return TBase64::Encode(buf.data(), nout); } diff --git a/tree/ntuple/inc/ROOT/RNTupleZip.hxx b/tree/ntuple/inc/ROOT/RNTupleZip.hxx index 77bbd5f217e68..94affb1f1e583 100644 --- a/tree/ntuple/inc/ROOT/RNTupleZip.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleZip.hxx @@ -57,7 +57,7 @@ public: auto cxAlgorithm = static_cast(compression / 100); unsigned int nZipBlocks = 1 + (nbytes - 1) / kMAXZIPBUF; - char *source = const_cast(static_cast(from)); + const char *source = static_cast(from); int szTarget = nbytes; char *target = reinterpret_cast(to); int szOutBlock = 0; @@ -111,7 +111,7 @@ public: } R__ASSERT(dataLen > nbytes); - unsigned char *source = const_cast(static_cast(from)); + const unsigned char *source = static_cast(from); unsigned char *target = static_cast(to); int szRemaining = dataLen; do { diff --git a/tree/tree/src/TTreeCacheUnzip.cxx b/tree/tree/src/TTreeCacheUnzip.cxx index 507a23d0e218c..098fd00001bb8 100644 --- a/tree/tree/src/TTreeCacheUnzip.cxx +++ b/tree/tree/src/TTreeCacheUnzip.cxx @@ -18,6 +18,7 @@ A TTreeCache which exploits parallelized decompression of its own content. */ +#include "RZip.h" #include "TTreeCacheUnzip.h" #include "TBranch.h" #include "TChain.h" @@ -35,9 +36,6 @@ A TTreeCache which exploits parallelized decompression of its own content. #include -extern "C" void R__unzip(Int_t *nin, UChar_t *bufin, Int_t *lout, char *bufout, Int_t *nout); -extern "C" int R__unzip_header(Int_t *nin, UChar_t *bufin, Int_t *lout); - TTreeCacheUnzip::EParUnzipMode TTreeCacheUnzip::fgParallel = TTreeCacheUnzip::kDisable; // The unzip cache does not consume memory by itself, it just allocates in advance @@ -911,7 +909,7 @@ Int_t TTreeCacheUnzip::UnzipBuffer(char **dest, char *src) return uzlen; } - R__unzip(&nin, bufcur, &nbuf, objbuf, &nout); + R__unzip(&nin, bufcur, &nbuf, reinterpret_cast(objbuf), &nout); if (gDebug > 2) Info("UnzipBuffer", "R__unzip nin:%d, bufcur:%p, nbuf:%d, objbuf:%p, nout:%d", From 3ad412b6c55c243f015fb8a5b64958bc02e8ef8e Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 17 Dec 2025 22:52:13 +0100 Subject: [PATCH 2/8] [core] define ROOT::Internal::kZipHeaderSize --- core/zip/inc/RZip.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/zip/inc/RZip.h b/core/zip/inc/RZip.h index 3da41c0914c0e..832d2fa5afe60 100644 --- a/core/zip/inc/RZip.h +++ b/core/zip/inc/RZip.h @@ -17,6 +17,12 @@ #ifndef ROOT_RZip #define ROOT_RZip +namespace ROOT::Internal { + +constexpr int kZipHeaderSize = 9; ///< Number of bytes of the ROOT compression block header + +} // namespace ROOT::Internal + extern "C" unsigned long R__crc32(unsigned long crc, const unsigned char *buf, unsigned int len); extern "C" unsigned long R__memcompress(char *tgt, unsigned long tgtsize, const char *src, unsigned long srcsize); From 900ff16d55ca69be4921f5d96927bdc1acb3e79d Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 17 Dec 2025 15:15:31 +0100 Subject: [PATCH 3/8] [io] add unit test for zip header handling --- io/io/test/CMakeLists.txt | 2 ++ io/io/test/ZipHeader.cxx | 15 +++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 io/io/test/ZipHeader.cxx diff --git a/io/io/test/CMakeLists.txt b/io/io/test/CMakeLists.txt index 8e1031f01a1e9..5036412810b6d 100644 --- a/io/io/test/CMakeLists.txt +++ b/io/io/test/CMakeLists.txt @@ -23,6 +23,8 @@ if(pyroot) ROOT_ADD_PYUNITTEST(rfile_py rfile.py) endif() +ROOT_ADD_GTEST(ZipHeader ZipHeader.cxx LIBRARIES RIO Tree ROOTNTuple) + # Temporarily disabled. Test routinely fails on MacOS and some Linuxes. #if(NOT WIN32 AND (NOT MACOS_VERSION OR NOT MACOSX_VERSION VERSION_LESS 13.00)) # ROOT_EXECUTABLE(TMapFileTest TMapFileTest.cxx LIBRARIES RIO Hist New) diff --git a/io/io/test/ZipHeader.cxx b/io/io/test/ZipHeader.cxx new file mode 100644 index 0000000000000..30620de03b1cb --- /dev/null +++ b/io/io/test/ZipHeader.cxx @@ -0,0 +1,15 @@ +#include +#include + +#include + +TEST(RZip, HeaderBasics) +{ + unsigned char header[9] = {'Z', 'S', '\1', 1, 0, 0, 2, 1, 0}; + int srcsize = 0; + int tgtsize = 0; + + EXPECT_EQ(0, R__unzip_header(&srcsize, header, &tgtsize)); + EXPECT_EQ(10, srcsize); + EXPECT_EQ(258, tgtsize); +} From 097e87b0d6c537e2d30a4868718af4555b620575 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 17 Dec 2025 15:14:17 +0100 Subject: [PATCH 4/8] [ntuple] fix handling of corrupt zip buffer --- io/io/test/ZipHeader.cxx | 89 +++++++++++++++++++++++++++++ tree/ntuple/inc/ROOT/RNTupleZip.hxx | 23 +++++--- 2 files changed, 105 insertions(+), 7 deletions(-) diff --git a/io/io/test/ZipHeader.cxx b/io/io/test/ZipHeader.cxx index 30620de03b1cb..84552709ece5a 100644 --- a/io/io/test/ZipHeader.cxx +++ b/io/io/test/ZipHeader.cxx @@ -1,8 +1,16 @@ #include #include +#include +#include + #include +#include + +using ROOT::Internal::RNTupleCompressor; +using ROOT::Internal::RNTupleDecompressor; + TEST(RZip, HeaderBasics) { unsigned char header[9] = {'Z', 'S', '\1', 1, 0, 0, 2, 1, 0}; @@ -13,3 +21,84 @@ TEST(RZip, HeaderBasics) EXPECT_EQ(10, srcsize); EXPECT_EQ(258, tgtsize); } + +TEST(RZip, CorruptHeaderRNTuple) +{ + constexpr char content[] = {7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7}; + unsigned char blocks[50]; + unsigned char verify[50]; + static_assert(sizeof(content) < sizeof(blocks)); + + const auto sz1 = RNTupleCompressor::Zip(content, sizeof(content), 101, blocks); + EXPECT_LT(sz1, sizeof(content)); + + RNTupleDecompressor::Unzip(blocks, sz1, sizeof(content), verify); + EXPECT_EQ(0, memcmp(content, verify, sizeof(content))); + + try { + RNTupleDecompressor::Unzip(blocks, 8, sizeof(content), verify); + FAIL() << "too short input buffer should throw"; + } catch (const ROOT::RException &e) { + EXPECT_THAT(e.what(), ::testing::HasSubstr("zip buffer too short")); + } + + try { + RNTupleDecompressor::Unzip(blocks, sz1 + 1, sizeof(content), verify); + FAIL() << "too long input buffer should throw"; + } catch (const ROOT::RException &e) { + EXPECT_THAT(e.what(), ::testing::HasSubstr("unexpected trailing bytes in zip buffer")); + } + + blocks[6]++; + try { + RNTupleDecompressor::Unzip(blocks, sz1, sizeof(content) + 1, verify); + FAIL() << "too long target size should throw"; + } catch (const ROOT::RException &e) { + EXPECT_THAT(e.what(), ::testing::HasSubstr("unexpected length after unzipping the buffer")); + } + blocks[6]--; + + + EXPECT_LT(sz1, sizeof(blocks) + 10); + blocks[sz1] = 'Z'; + blocks[sz1 + 1] = 'S'; + blocks[sz1 + 2] = '\1'; + blocks[sz1 + 3] = 0; + blocks[sz1 + 4] = 0; + blocks[sz1 + 5] = 0; + blocks[sz1 + 6] = 0; + blocks[sz1 + 7] = 0; + blocks[sz1 + 8] = 0; + try { + RNTupleDecompressor::Unzip(blocks, sz1 + 10, sizeof(verify), verify); + FAIL() << "source length zero in the header should fail"; + } catch (const ROOT::RException &e) { + EXPECT_THAT(e.what(), ::testing::HasSubstr("failed to unzip buffer header")); + } + + blocks[sz1 + 3] = 1; + try { + RNTupleDecompressor::Unzip(blocks, sz1 + 10, sizeof(verify), verify); + FAIL() << "source size < target size should fail"; + } catch (const ROOT::RException &e) { + EXPECT_THAT(e.what(), ::testing::HasSubstr("failed to unzip buffer header")); + } + + blocks[sz1 + 3] = 11; + blocks[sz1 + 6] = 12; + try { + RNTupleDecompressor::Unzip(blocks, sz1 + 10, sizeof(verify), verify); + FAIL() << "too big source size should fail"; + } catch (const ROOT::RException &e) { + EXPECT_THAT(e.what(), ::testing::HasSubstr("failed to unzip buffer header")); + } + + blocks[sz1 + 3] = 1; + blocks[sz1 + 6] = sizeof(verify); + try { + RNTupleDecompressor::Unzip(blocks, sz1 + 10, sizeof(verify), verify); + FAIL() << "too big target size should fail"; + } catch (const ROOT::RException &e) { + EXPECT_THAT(e.what(), ::testing::HasSubstr("failed to unzip buffer header")); + } +} diff --git a/tree/ntuple/inc/ROOT/RNTupleZip.hxx b/tree/ntuple/inc/ROOT/RNTupleZip.hxx index 94affb1f1e583..7bbb3d0623987 100644 --- a/tree/ntuple/inc/ROOT/RNTupleZip.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleZip.hxx @@ -113,28 +113,37 @@ public: const unsigned char *source = static_cast(from); unsigned char *target = static_cast(to); - int szRemaining = dataLen; + size_t szRemainingLen = dataLen; + size_t szRemainingNbytes = nbytes; do { + if (R__unlikely(szRemainingNbytes < ROOT::Internal::kZipHeaderSize)) { + throw ROOT::RException(R__FAIL("zip buffer too short")); + } int szSource; int szTarget; int retval = R__unzip_header(&szSource, source, &szTarget); if (R__unlikely(!((retval == 0) && (szSource > 0) && (szTarget > szSource) && - (static_cast(szSource) <= nbytes) && - (static_cast(szTarget) <= dataLen)))) { + (static_cast(szSource) <= szRemainingNbytes) && + (static_cast(szTarget) <= szRemainingLen)))) { throw ROOT::RException(R__FAIL("failed to unzip buffer header")); } int unzipBytes = 0; R__unzip(&szSource, source, &szTarget, target, &unzipBytes); - if (R__unlikely(unzipBytes != szTarget)) + if (R__unlikely(unzipBytes != szTarget)) { throw ROOT::RException(R__FAIL(std::string("unexpected length after unzipping the buffer (wanted: ") + std::to_string(szTarget) + ", got: " + std::to_string(unzipBytes) + ")")); + } target += szTarget; source += szSource; - szRemaining -= unzipBytes; - } while (szRemaining > 0); - R__ASSERT(szRemaining == 0); + szRemainingNbytes -= szSource; + szRemainingLen -= unzipBytes; + } while (szRemainingLen > 0); + R__ASSERT(szRemainingLen == 0); + if (szRemainingNbytes > 0) { + throw ROOT::RException(R__FAIL(std::string("unexpected trailing bytes in zip buffer"))); + } } }; From 1a74f25479961006a71e1ec05843a1004e8a0184 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 18 Dec 2025 00:04:27 +0100 Subject: [PATCH 5/8] [io] add TKey::UnzipObject() Factor out the payload decompression in the different variants of reading a key. --- io/io/inc/TKey.h | 2 ++ io/io/src/TKey.cxx | 90 +++++++++++++++++----------------------------- 2 files changed, 35 insertions(+), 57 deletions(-) diff --git a/io/io/inc/TKey.h b/io/io/inc/TKey.h index a94081326ada1..40991abea39a0 100644 --- a/io/io/inc/TKey.h +++ b/io/io/inc/TKey.h @@ -35,6 +35,8 @@ class TKey : public TNamed { TKey(const TKey&) = delete; // TKey objects are not copiable. TKey& operator=(const TKey&) = delete; // TKey objects are not copiable. + Int_t UnzipObject(char *targetBuffer, const char *compressedBuffer) const; + protected: Int_t fVersion; ///< Key version identifier Int_t fNbytes; ///< Number of bytes for the object on file diff --git a/io/io/src/TKey.cxx b/io/io/src/TKey.cxx index 2567681d839bc..0eea166e965c1 100644 --- a/io/io/src/TKey.cxx +++ b/io/io/src/TKey.cxx @@ -382,6 +382,33 @@ TKey::TKey(const void *obj, const TClass *cl, const char *name, Int_t bufsize, T } } +/// Core of uncompressing the key payload. Returns number of bytes uncompressed. +Int_t TKey::UnzipObject(char *targetBuffer, const char *compressedBuffer) const +{ + auto objbuf = reinterpret_cast(targetBuffer) + fKeylen; + auto bufcur = reinterpret_cast(&compressedBuffer[fKeylen]); + Int_t nin, nout = 0, nbuf; + Int_t noutot = 0; + Int_t nbytesRemain = fNbytes - fKeylen; + Int_t objlenRemain = fObjlen; + while (nbytesRemain >= ROOT::Internal::kZipHeaderSize) { + Int_t hc = R__unzip_header(&nin, bufcur, &nbuf); + if ((hc != 0) || (nin > nbytesRemain) || (nbuf > objlenRemain)) + return 0; + R__unzip(&nin, bufcur, &nbuf, objbuf, &nout); + if (!nout) + return 0; + noutot += nout; + if (noutot >= fObjlen) + break; + bufcur += nin; + objbuf += nout; + nbytesRemain -= nin; + objlenRemain -= nout; + } + return nout; +} + //////////////////////////////////////////////////////////////////////////////// /// Method used in all TKey constructor to initialize basic data fields. /// @@ -825,20 +852,7 @@ TObject *TKey::ReadObj() bufferRef.MapObject(pobj,cl); //register obj in map to handle self reference if (fObjlen > fNbytes-fKeylen) { - char *objbuf = bufferRef.Buffer() + fKeylen; - UChar_t *bufcur = (UChar_t *)&compressedBuffer[fKeylen]; - Int_t nin, nout = 0, nbuf; - Int_t noutot = 0; - while (1) { - Int_t hc = R__unzip_header(&nin, bufcur, &nbuf); - if (hc!=0) break; - R__unzip(&nin, bufcur, &nbuf, (unsigned char*) objbuf, &nout); - if (!nout) break; - noutot += nout; - if (noutot >= fObjlen) break; - bufcur += nin; - objbuf += nout; - } + Int_t nout = UnzipObject(bufferRef.Buffer(), compressedBuffer.get()); compressedBuffer.reset(nullptr); if (nout) { tobj->Streamer(bufferRef); //does not work with example 2 above @@ -950,20 +964,7 @@ TObject *TKey::ReadObjWithBuffer(char *bufferRead) bufferRef.MapObject(pobj,cl); //register obj in map to handle self reference if (fObjlen > fNbytes-fKeylen) { - char *objbuf = bufferRef.Buffer() + fKeylen; - UChar_t *bufcur = (UChar_t *)&bufferRead[fKeylen]; - Int_t nin, nout = 0, nbuf; - Int_t noutot = 0; - while (1) { - Int_t hc = R__unzip_header(&nin, bufcur, &nbuf); - if (hc!=0) break; - R__unzip(&nin, bufcur, &nbuf, (unsigned char*) objbuf, &nout); - if (!nout) break; - noutot += nout; - if (noutot >= fObjlen) break; - bufcur += nin; - objbuf += nout; - } + Int_t nout = UnzipObject(bufferRef.Buffer(), bufferRead); if (nout) { tobj->Streamer(bufferRef); //does not work with example 2 above } else { @@ -1096,20 +1097,7 @@ void *TKey::ReadObjectAny(const TClass* expectedClass) bufferRef.MapObject(pobj,cl); //register obj in map to handle self reference if (fObjlen > fNbytes-fKeylen) { - char *objbuf = bufferRef.Buffer() + fKeylen; - UChar_t *bufcur = (UChar_t *)&compressedBuffer[fKeylen]; - Int_t nin, nout = 0, nbuf; - Int_t noutot = 0; - while (1) { - Int_t hc = R__unzip_header(&nin, bufcur, &nbuf); - if (hc!=0) break; - R__unzip(&nin, bufcur, &nbuf, (unsigned char*) objbuf, &nout); - if (!nout) break; - noutot += nout; - if (noutot >= fObjlen) break; - bufcur += nin; - objbuf += nout; - } + Int_t nout = UnzipObject(bufferRef.Buffer(), compressedBuffer.get()); if (nout) { cl->Streamer((void*)pobj, bufferRef, clOnfile); //read object } else { @@ -1184,21 +1172,9 @@ Int_t TKey::Read(TObject *obj) bufferRef.SetBufferOffset(fKeylen); if (fObjlen > fNbytes-fKeylen) { - char *objbuf = bufferRef.Buffer() + fKeylen; - UChar_t *bufcur = (UChar_t *)&compressedBuffer[fKeylen]; - Int_t nin, nout = 0, nbuf; - Int_t noutot = 0; - while (1) { - Int_t hc = R__unzip_header(&nin, bufcur, &nbuf); - if (hc!=0) break; - R__unzip(&nin, bufcur, &nbuf, (unsigned char*) objbuf, &nout); - if (!nout) break; - noutot += nout; - if (noutot >= fObjlen) break; - bufcur += nin; - objbuf += nout; - } - if (nout) obj->Streamer(bufferRef); + Int_t nout = UnzipObject(bufferRef.Buffer(), compressedBuffer.get()); + if (nout) + obj->Streamer(bufferRef); } else { obj->Streamer(bufferRef); } From 9d4696f6bcf715630a6037a990d25ad0a58c9109 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 18 Dec 2025 00:11:24 +0100 Subject: [PATCH 6/8] [ntuple] add test for unzip header handling in TKey --- io/io/test/ZipHeader.cxx | 60 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/io/io/test/ZipHeader.cxx b/io/io/test/ZipHeader.cxx index 84552709ece5a..05c86ad7133bc 100644 --- a/io/io/test/ZipHeader.cxx +++ b/io/io/test/ZipHeader.cxx @@ -5,8 +5,13 @@ #include #include +#include +#include +#include #include +#include +#include using ROOT::Internal::RNTupleCompressor; using ROOT::Internal::RNTupleDecompressor; @@ -102,3 +107,58 @@ TEST(RZip, CorruptHeaderRNTuple) EXPECT_THAT(e.what(), ::testing::HasSubstr("failed to unzip buffer header")); } } + +TEST(RZip, CorruptHeaderTKey) +{ + TMemFile writableFile("memfile.root", "RECREATE"); + writableFile.SetCompressionSettings(101); + + std::string stdstring(1000, 'x'); + TNamed tnamed; + tnamed.SetName(stdstring.c_str()); + writableFile.WriteObject(&stdstring, "stdstring"); + writableFile.WriteObject(&tnamed, "tnamed"); + + auto keysInfo = writableFile.WalkTKeys(); + std::size_t posZipStdString = 0; + std::size_t posZipTNamed = 0; + for (const auto &ki : keysInfo) { + if (ki.fKeyName == "stdstring") { + EXPECT_LT(ki.fLen, ki.fObjLen); // ensure it's compressed + posZipStdString = ki.fSeekKey + ki.fKeyLen; + } else if (ki.fKeyName == "tnamed") { + EXPECT_LT(ki.fLen, ki.fObjLen); // ensure it's compressed + posZipTNamed = ki.fSeekKey + ki.fKeyLen; + } + } + EXPECT_GT(posZipStdString, 0); + EXPECT_GT(posZipTNamed, 0); + + writableFile.Close(); + + auto buffer = std::make_unique(writableFile.GetSize()); + writableFile.CopyTo(buffer.get(), writableFile.GetSize()); + + TMemFile verifyFile("memfile.root", TMemFile::ZeroCopyView_t(buffer.get(), writableFile.GetSize())); + + buffer[posZipStdString + 3]++; + EXPECT_FALSE(verifyFile.Get("stdstring")); + buffer[posZipStdString + 3]--; + buffer[posZipStdString + 6]++; + EXPECT_FALSE(verifyFile.Get("stdstring")); + buffer[posZipStdString + 6]--; + + auto k = verifyFile.GetKey("tnamed"); + EXPECT_TRUE(k); + EXPECT_TRUE(dynamic_cast(k->ReadObj())); + + buffer[posZipTNamed + 3]++; + EXPECT_FALSE(dynamic_cast(k->ReadObj())); + buffer[posZipTNamed + 3]--; + buffer[posZipTNamed + 6]++; + EXPECT_FALSE(dynamic_cast(k->ReadObj())); + buffer[posZipTNamed + 6]--; + + EXPECT_TRUE(verifyFile.Get("stdstring")); + EXPECT_TRUE(verifyFile.Get("tnamed")); +} From 7e90e8862286134bdb732dda1b9eb1581f286527 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 18 Dec 2025 14:41:22 +0100 Subject: [PATCH 7/8] [tree] fix handling of corrupt zip buffer in TBasket --- io/io/test/ZipHeader.cxx | 53 +++++++++++++++++++++++++++++++++++++++ tree/tree/src/TBasket.cxx | 12 ++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/io/io/test/ZipHeader.cxx b/io/io/test/ZipHeader.cxx index 05c86ad7133bc..1378c0c183da0 100644 --- a/io/io/test/ZipHeader.cxx +++ b/io/io/test/ZipHeader.cxx @@ -3,11 +3,13 @@ #include #include +#include #include #include #include #include +#include #include #include @@ -162,3 +164,54 @@ TEST(RZip, CorruptHeaderTKey) EXPECT_TRUE(verifyFile.Get("stdstring")); EXPECT_TRUE(verifyFile.Get("tnamed")); } + +TEST(RZip, CorruptHeaderTBasket) +{ + TMemFile writableFile("memfile.root", "RECREATE"); + writableFile.SetCompressionSettings(101); + auto tree = new TTree("t", ""); + int val = 137; + tree->Branch("val", &val); + for (int i = 0; i < 1000; ++i) + tree->Fill(); + tree->Write(); + + auto keysInfo = writableFile.WalkTKeys(); + std::size_t posTBasket = 0; + for (const auto &ki : keysInfo) { + if (ki.fClassName != "TBasket") + continue; + + EXPECT_EQ(0u, posTBasket); // We expect only one basket + EXPECT_LT(ki.fLen, ki.fObjLen); // ensure it's compressed + posTBasket = ki.fSeekKey + ki.fKeyLen; + } + EXPECT_GT(posTBasket, 0); + + writableFile.Close(); + + auto buffer = std::make_unique(writableFile.GetSize()); + writableFile.CopyTo(buffer.get(), writableFile.GetSize()); + + for (int headerOffset : {3, 6}) { + TMemFile verifyFile("memfile.root", TMemFile::ZeroCopyView_t(buffer.get(), writableFile.GetSize())); + + tree = verifyFile.Get("t"); + + ROOT::TestSupport::CheckDiagsRAII checkDiag; + checkDiag.requiredDiag(kError, "TBasket::ReadBasketBuffers", "fNbytes", /* matchFullMessage= */ false); + checkDiag.requiredDiag(kError, "TBranch::GetBasket", "File: memfile.root", /* matchFullMessage= */ false); + + buffer[posTBasket + headerOffset]++; + EXPECT_EQ(-1, tree->GetEntry(0)); + buffer[posTBasket + headerOffset]--; + } + + TMemFile verifyFile("memfile.root", TMemFile::ZeroCopyView_t(buffer.get(), writableFile.GetSize())); + tree = verifyFile.Get("t"); + tree->SetBranchAddress("val", &val); + val = 0; + + EXPECT_EQ(sizeof(int), tree->GetEntry(0)); + EXPECT_EQ(137, val); +} diff --git a/tree/tree/src/TBasket.cxx b/tree/tree/src/TBasket.cxx index e86e1de93b8c6..aa6f7025e0f33 100644 --- a/tree/tree/src/TBasket.cxx +++ b/tree/tree/src/TBasket.cxx @@ -601,11 +601,16 @@ Int_t TBasket::ReadBasketBuffers(Long64_t pos, Int_t len, TFile *file) memcpy(rawUncompressedBuffer, rawCompressedBuffer, fKeylen); char *rawUncompressedObjectBuffer = rawUncompressedBuffer+fKeylen; UChar_t *rawCompressedObjectBuffer = (UChar_t*)rawCompressedBuffer+fKeylen; + + // TODO(jblomer): factor out and combine with UnzipObject() in TKey Int_t nin, nbuf; Int_t nout = 0, noutot = 0, nintot = 0; + Int_t nbytesRemain = fNbytes - fKeylen; + Int_t objlenRemain = fObjlen; + // Unzip all the compressed objects in the compressed object buffer. - while (true) { + while (nbytesRemain >= ROOT::Internal::kZipHeaderSize) { // Check the header for errors. if (R__unlikely(R__unzip_header(&nin, rawCompressedObjectBuffer, &nbuf) != 0)) { Error("ReadBasketBuffers", "Inconsistency found in header (nin=%d, nbuf=%d)", nin, nbuf); @@ -616,6 +621,9 @@ Int_t TBasket::ReadBasketBuffers(Long64_t pos, Int_t len, TFile *file) memcpy(rawUncompressedBuffer+fKeylen, rawCompressedObjectBuffer+fKeylen, fObjlen); goto AfterBuffer; } + if (R__unlikely((nin > nbytesRemain) || (nbuf > objlenRemain))) { + break; + } R__unzip(&nin, rawCompressedObjectBuffer, &nbuf, (unsigned char*) rawUncompressedObjectBuffer, &nout); if (!nout) break; @@ -624,6 +632,8 @@ Int_t TBasket::ReadBasketBuffers(Long64_t pos, Int_t len, TFile *file) if (noutot >= fObjlen) break; rawCompressedObjectBuffer += nin; rawUncompressedObjectBuffer += nout; + nbytesRemain -= nin; + objlenRemain -= nout; } // Make sure the uncompressed numbers are consistent with header. From 5034e3ab299571fd11240208418ca854ef4c5d55 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 19 Dec 2025 09:55:37 +0100 Subject: [PATCH 8/8] [tree] fix handling of corrupt zip buffer in TTreeCacheUnzip --- io/io/test/ZipHeader.cxx | 26 +++++++++++++++++++++++++- tree/tree/src/TTreeCacheUnzip.cxx | 13 ++++++++++--- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/io/io/test/ZipHeader.cxx b/io/io/test/ZipHeader.cxx index 1378c0c183da0..c1c0d17512ef8 100644 --- a/io/io/test/ZipHeader.cxx +++ b/io/io/test/ZipHeader.cxx @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -165,7 +166,7 @@ TEST(RZip, CorruptHeaderTKey) EXPECT_TRUE(verifyFile.Get("tnamed")); } -TEST(RZip, CorruptHeaderTBasket) +TEST(RZip, CorruptHeaderTTree) { TMemFile writableFile("memfile.root", "RECREATE"); writableFile.SetCompressionSettings(101); @@ -178,6 +179,7 @@ TEST(RZip, CorruptHeaderTBasket) auto keysInfo = writableFile.WalkTKeys(); std::size_t posTBasket = 0; + std::size_t keylenTBasket = 0; for (const auto &ki : keysInfo) { if (ki.fClassName != "TBasket") continue; @@ -185,6 +187,7 @@ TEST(RZip, CorruptHeaderTBasket) EXPECT_EQ(0u, posTBasket); // We expect only one basket EXPECT_LT(ki.fLen, ki.fObjLen); // ensure it's compressed posTBasket = ki.fSeekKey + ki.fKeyLen; + keylenTBasket = ki.fKeyLen; } EXPECT_GT(posTBasket, 0); @@ -207,6 +210,27 @@ TEST(RZip, CorruptHeaderTBasket) buffer[posTBasket + headerOffset]--; } + { + TMemFile verifyFile("memfile.root", TMemFile::ZeroCopyView_t(buffer.get(), writableFile.GetSize())); + + tree = verifyFile.Get("t"); + + TTreeCacheUnzip cache(tree); + char *dest = nullptr; + + for (int headerOffset : {3, 6}) { + ROOT::TestSupport::CheckDiagsRAII checkDiag; + checkDiag.requiredDiag(kError, "TTreeCacheUnzip::UnzipBuffer", "nbytes", /* matchFullMessage= */ false); + + buffer[posTBasket + headerOffset]++; + EXPECT_EQ(-1, cache.UnzipBuffer(&dest, &buffer[posTBasket - keylenTBasket])); + buffer[posTBasket + headerOffset]--; + } + + EXPECT_GT(cache.UnzipBuffer(&dest, &buffer[posTBasket - keylenTBasket]), 0); + delete[] dest; + } + TMemFile verifyFile("memfile.root", TMemFile::ZeroCopyView_t(buffer.get(), writableFile.GetSize())); tree = verifyFile.Get("t"); tree->SetBranchAddress("val", &val); diff --git a/tree/tree/src/TTreeCacheUnzip.cxx b/tree/tree/src/TTreeCacheUnzip.cxx index 098fd00001bb8..4924c423bda34 100644 --- a/tree/tree/src/TTreeCacheUnzip.cxx +++ b/tree/tree/src/TTreeCacheUnzip.cxx @@ -858,11 +858,15 @@ Int_t TTreeCacheUnzip::UnzipBuffer(char **dest, char *src) Int_t nbytes = 0, objlen = 0, keylen = 0; GetRecordHeader(src, hlen, nbytes, objlen, keylen); + Int_t nbytesRemain = nbytes - keylen; + Int_t objlenRemain = objlen; + if (!(*dest)) { /* early consistency check */ UChar_t *bufcur = (UChar_t *) (src + keylen); Int_t nin, nbuf; - if(objlen > nbytes - keylen && R__unzip_header(&nin, bufcur, &nbuf) != 0) { + if((objlen > nbytes - keylen) && + ((nbytesRemain < ROOT::Internal::kZipHeaderSize) || (R__unzip_header(&nin, bufcur, &nbuf) != 0))) { Error("UnzipBuffer", "Inconsistency found in header (nin=%d, nbuf=%d)", nin, nbuf); uzlen = -1; return uzlen; @@ -893,9 +897,10 @@ Int_t TTreeCacheUnzip::UnzipBuffer(char **dest, char *src) Int_t nout = 0; Int_t noutot = 0; - while (true) { + while (nbytesRemain >= ROOT::Internal::kZipHeaderSize) { Int_t hc = R__unzip_header(&nin, bufcur, &nbuf); - if (hc != 0) break; + if ((hc != 0) || (nin > nbytesRemain) || (nbuf > objlenRemain)) + break; if (gDebug > 2) Info("UnzipBuffer", " nin:%d, nbuf:%d, bufcur[3] :%d, bufcur[4] :%d, bufcur[5] :%d ", nin, nbuf, bufcur[3], bufcur[4], bufcur[5]); @@ -920,6 +925,8 @@ Int_t TTreeCacheUnzip::UnzipBuffer(char **dest, char *src) if (noutot >= objlen) break; bufcur += nin; objbuf += nout; + nbytesRemain -= nin; + objlenRemain -= nout; } if (noutot != objlen) {