Skip to content

Commit

Permalink
[io] prevent infinite loops and add more security for overflows
Browse files Browse the repository at this point in the history 
Fixes #14770
  • Loading branch information
@ferdymercury @vepadulano
ferdymercury authored and vepadulano committed Mar 4, 2024
1 parent 1df0dc6 commit d806bb1
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 16 deletions.
16 changes: 12 additions & 4 deletions io/io/src/TBufferJSON.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>() 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 <typename T>
void TBufferJSON::JsonWriteFastArray(const T *arr, Long64_t arrsize, const char *typname,
void (TBufferJSON::*method)(const T *, Int_t, const char *))
Expand All @@ -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<Int_t>::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)) {
Expand Down Expand Up @@ -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<n;++i) {
for (Long64_t i=0;i<n;++i) {
if (!c[i]) {
has_zero = true; // might be terminal '\0'
} else if (has_zero || !isprint(c[i])) {
Expand Down Expand Up @@ -3450,7 +3458,7 @@ void TBufferJSON::WriteFastArray(void *start, const TClass *cl, Long64_t n, TMem
AppendOutput(indexes.GetBegin());
}

for (Int_t j = 0; j < n; j++, obj += size) {
for (Long64_t j = 0; j < n; j++, obj += size) {

if (j > 0)
AppendOutput(indexes.NextSeparator());
Expand Down Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion io/sql/inc/TBufferSQL2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class TBufferSQL2 final : public TBufferText {
R__ALWAYS_INLINE void SqlReadFastArray(T *arr, Int_t arrsize);

template <typename T>
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);
Expand Down
19 changes: 13 additions & 6 deletions io/sql/src/TBufferSQL2.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1374,8 +1374,15 @@ Int_t TBufferSQL2::SqlReadArraySize()
}

template <typename T>
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<Int_t>::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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
18 changes: 13 additions & 5 deletions io/xml/src/TBufferXML.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename T>
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<Int_t>::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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
Loading

0 comments on commit d806bb1

Please sign in to comment.