From d806bb16d80b5b737b4bf4abc1c6817ab4fe7231 Mon Sep 17 00:00:00 2001 From: ferdymercury Date: Sun, 25 Feb 2024 19:09:16 +0100 Subject: [PATCH] [io] prevent infinite loops and add more security for overflows Fixes https://github.com/root-project/root/issues/14770 --- io/io/src/TBufferJSON.cxx | 16 +++-- io/sql/inc/TBufferSQL2.h | 2 +- io/sql/src/TBufferSQL2.cxx | 19 ++++-- io/xml/src/TBufferXML.cxx | 18 ++++-- tree/tree/src/TBufferSQL.cxx | 115 +++++++++++++++++++++++++++++++++++ 5 files changed, 154 insertions(+), 16 deletions(-) diff --git a/io/io/src/TBufferJSON.cxx b/io/io/src/TBufferJSON.cxx index f38b4c34b8116..9076f5fdcaf85 100644 --- a/io/io/src/TBufferJSON.cxx +++ b/io/io/src/TBufferJSON.cxx @@ -3252,7 +3252,8 @@ void TBufferJSON::WriteArray(const Double_t *d, Int_t n) /// Template method to write array of arbitrary dimensions /// Different methods can be used for store last array dimension - /// either JsonWriteArrayCompress() or JsonWriteConstChar() - +/// \note Due to the current limit of the buffer size, the function aborts execution of the program in case of overflow. See https://github.com/root-project/root/issues/6734 for more details. +/// template void TBufferJSON::JsonWriteFastArray(const T *arr, Long64_t arrsize, const char *typname, void (TBufferJSON::*method)(const T *, Int_t, const char *)) @@ -3262,6 +3263,13 @@ void TBufferJSON::JsonWriteFastArray(const T *arr, Long64_t arrsize, const char fValue.Append("[]"); return; } + constexpr Int_t dataWidth = 1; // at least 1 + const Int_t maxElements = (std::numeric_limits::max() - Length())/dataWidth; + if (arrsize > maxElements) + { + Fatal("JsonWriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", arrsize, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } TStreamerElement *elem = Stack()->fElem; if (elem && (elem->GetArrayDim() > 1) && (elem->GetArrayLength() == arrsize)) { @@ -3307,7 +3315,7 @@ void TBufferJSON::WriteFastArray(const Char_t *c, Long64_t n) { Bool_t need_blob = false; Bool_t has_zero = false; - for (int i=0;i 0) AppendOutput(indexes.NextSeparator()); @@ -3498,7 +3506,7 @@ Int_t TBufferJSON::WriteFastArray(void **start, const TClass *cl, Long64_t n, Bo AppendOutput(indexes.GetBegin()); } - for (Int_t j = 0; j < n; j++) { + for (Long64_t j = 0; j < n; j++) { if (j > 0) AppendOutput(indexes.NextSeparator()); diff --git a/io/sql/inc/TBufferSQL2.h b/io/sql/inc/TBufferSQL2.h index f2e85e41c02d4..1c7c7e696adae 100644 --- a/io/sql/inc/TBufferSQL2.h +++ b/io/sql/inc/TBufferSQL2.h @@ -119,7 +119,7 @@ class TBufferSQL2 final : public TBufferText { R__ALWAYS_INLINE void SqlReadFastArray(T *arr, Int_t arrsize); template - R__ALWAYS_INLINE void SqlWriteArray(T *arr, Int_t arrsize, Bool_t withsize = kFALSE); + R__ALWAYS_INLINE void SqlWriteArray(T *arr, Long64_t arrsize, Bool_t withsize = kFALSE); public: TBufferSQL2(TBuffer::EMode mode, TSQLFile *file = nullptr); diff --git a/io/sql/src/TBufferSQL2.cxx b/io/sql/src/TBufferSQL2.cxx index c4e646d98f9cc..59cb164b3925c 100644 --- a/io/sql/src/TBufferSQL2.cxx +++ b/io/sql/src/TBufferSQL2.cxx @@ -1374,8 +1374,15 @@ Int_t TBufferSQL2::SqlReadArraySize() } template -R__ALWAYS_INLINE void TBufferSQL2::SqlWriteArray(T *arr, Int_t arrsize, Bool_t withsize) -{ +R__ALWAYS_INLINE void TBufferSQL2::SqlWriteArray(T *arr, Long64_t arrsize, Bool_t withsize) +{ + constexpr Int_t dataWidth = 1; // at least 1 + const Int_t maxElements = (std::numeric_limits::max() - Length())/dataWidth; + if (arrsize < 0 || arrsize > maxElements) + { + Fatal("SqlWriteArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", arrsize, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } if (!withsize && (arrsize <= 0)) return; PushStack()->SetArray(withsize ? arrsize : -1); @@ -1520,7 +1527,7 @@ void TBufferSQL2::WriteFastArray(const Char_t *c, Long64_t n) const Char_t *ccc = c; // check if no zeros in the array if (!usedefault) - for (int i = 0; i < n; i++) + for (Long64_t i = 0; i < n; i++) if (*ccc++ == 0) { usedefault = kTRUE; break; @@ -1653,7 +1660,7 @@ void TBufferSQL2::WriteFastArray(void *start, const TClass *cl, Long64_t n, TMem n = 1; int size = cl->Size(); - for (Int_t j = 0; j < n; j++, obj += size) + for (Long64_t j = 0; j < n; j++, obj += size) StreamObject(obj, cl); } @@ -1689,7 +1696,7 @@ Int_t TBufferSQL2::WriteFastArray(void **start, const TClass *cl, Long64_t n, Bo if (!isPreAlloc) { - for (Int_t j = 0; j < n; j++) { + for (Long64_t j = 0; j < n; j++) { // must write StreamerInfo if pointer is null if (!strInfo && !start[j] && !oldStyle) ForceWriteInfo(((TClass *)cl)->GetStreamerInfo(), kFALSE); @@ -1703,7 +1710,7 @@ Int_t TBufferSQL2::WriteFastArray(void **start, const TClass *cl, Long64_t n, Bo } else { // case //-> in comment - for (Int_t j = 0; j < n; j++) { + for (Long64_t j = 0; j < n; j++) { if (!start[j]) start[j] = ((TClass *)cl)->New(); StreamObject(start[j], cl); diff --git a/io/xml/src/TBufferXML.cxx b/io/xml/src/TBufferXML.cxx index f6c615b283b3b..3eef4613b12af 100644 --- a/io/xml/src/TBufferXML.cxx +++ b/io/xml/src/TBufferXML.cxx @@ -2155,10 +2155,18 @@ void TBufferXML::WriteArray(const Double_t *d, Int_t n) /// Write array without size attribute /// Also treat situation, when instead of one single array /// chain of several elements should be produced - +/// \note Due to the current limit of the buffer size, the function aborts execution of the program in case of underflow or overflow. See https://github.com/root-project/root/issues/6734 for more details. +/// template R__ALWAYS_INLINE void TBufferXML::XmlWriteFastArray(const T *arr, Long64_t n) { + constexpr Int_t dataWidth = 1; // at least 1 + const Int_t maxElements = (std::numeric_limits::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("XmlWriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } BeforeIOoperation(); if (n <= 0) return; @@ -2186,7 +2194,7 @@ void TBufferXML::WriteFastArray(const Char_t *c, Long64_t n) Bool_t usedefault = (n == 0); const Char_t *buf = c; if (!usedefault) - for (int i = 0; i < n; i++) { + for (Long64_t i = 0; i < n; i++) { if (*buf < 27) { usedefault = kTRUE; break; @@ -2317,7 +2325,7 @@ void TBufferXML::WriteFastArray(void *start, const TClass *cl, Long64_t n, TMemb n = 1; int size = cl->Size(); - for (Int_t j = 0; j < n; j++, obj += size) { + for (Long64_t j = 0; j < n; j++, obj += size) { ((TClass *)cl)->Streamer(obj, *this); } } @@ -2355,7 +2363,7 @@ Int_t TBufferXML::WriteFastArray(void **start, const TClass *cl, Long64_t n, Boo if (!isPreAlloc) { - for (Int_t j = 0; j < n; j++) { + for (Long64_t j = 0; j < n; j++) { // must write StreamerInfo if pointer is null if (!strInfo && !start[j] && !oldStyle) { if (cl->Property() & kIsAbstract) { @@ -2375,7 +2383,7 @@ Int_t TBufferXML::WriteFastArray(void **start, const TClass *cl, Long64_t n, Boo } else { // case //-> in comment - for (Int_t j = 0; j < n; j++) { + for (Long64_t j = 0; j < n; j++) { if (!start[j]) start[j] = ((TClass *)cl)->New(); ((TClass *)cl)->Streamer(start[j], *this); diff --git a/tree/tree/src/TBufferSQL.cxx b/tree/tree/src/TBufferSQL.cxx index eaf1a4c7b7687..d552bcd8a3e64 100644 --- a/tree/tree/src/TBufferSQL.cxx +++ b/tree/tree/src/TBufferSQL.cxx @@ -422,9 +422,17 @@ void TBufferSQL::WriteCharP(const Char_t *str) //////////////////////////////////////////////////////////////////////////////// /// WriteFastArray SQL implementation. +/// \note Due to the current limit of the buffer size, the function aborts execution of the program in case of underflow or overflow. See https://github.com/root-project/root/issues/6734 for more details. void TBufferSQL::WriteFastArray(const bool *b, Long64_t n) { + constexpr Int_t dataWidth = 2; // 2 chars + const Int_t maxElements = (std::numeric_limits::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (maxElements < 1) + { + Fatal("WriteFastArrayString", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", 1LL, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } (*fInsertQuery) += "\""; (*fInsertQuery) += c; (*fInsertQuery) += "\","; @@ -457,9 +481,17 @@ void TBufferSQL::WriteFastArrayString(const Char_t *c, Long64_t /* n */) //////////////////////////////////////////////////////////////////////////////// /// WriteFastArray SQL implementation. +/// \note Due to the current limit of the buffer size, the function aborts execution of the program in case of underflow or overflow. See https://github.com/root-project/root/issues/6734 for more details. void TBufferSQL::WriteFastArray(const UChar_t *uc, Long64_t n) { + constexpr Int_t dataWidth = 2; // at least 2 chars, but could be more. + const Int_t maxElements = (std::numeric_limits::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } // std::cerr << "Column: " <<*fIter << " i:" << *ii << std::endl; for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i::max() - Length())/dataWidth; + if (n < 0 || n > maxElements) + { + Fatal("WriteFastArray", "Not enough space left in the buffer (1GB limit). %lld elements is greater than the max left of %d", n, maxElements); + return; // In case the user re-routes the error handler to not die when Fatal is called) + } for(int i=0; i