Skip to content

Commit 6ab4902

Browse files
dpiparodpiparo
committed
[io] TBufferFile: more checks when reading arrays
i.e. in the TBufferFile::Read*Array* methods. Previously, the collection length was checked to be positive and that it was less than the total buffer size. Those checks have been improved with a battery of 3 checks: 1. The collection's number of elements is checked to be positive 2. The collection length is checked to be positive 3. The collection length is checked to be less than the portion of the buffer that remains to be read The test roottest-root-io-prefetching-run had to be modified as well because of what seems to be a corrupted input file. thanks also to @Sebasteuo
1 parent 619d396 commit 6ab4902

2 files changed

Lines changed: 46 additions & 35 deletions

File tree

‎io/io/inc/TBufferFile.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ class TBufferFile : public TBufferIO {
6464
UInt_t CheckObject(UInt_t offset, const TClass *cl, Bool_t readClass = kFALSE);
6565

6666
void WriteObjectClass(const void *actualObjStart, const TClass *actualClass, Bool_t cacheReuse) override;
67+
bool ShouldNotReadCollection(Int_t lengthInBytes, Int_t nElements=1) const;
6768

6869
public:
6970
enum { kStreamedMemberWise = BIT(14) }; //added to version number to know if a collection has been stored member-wise
@@ -268,6 +269,16 @@ class TBufferFile : public TBufferIO {
268269

269270
//---------------------- TBufferFile inlines ---------------------------------------
270271

272+
//______________________________________________________________________________
273+
inline bool TBufferFile::ShouldNotReadCollection(Int_t lengthInBytes, Int_t nElements) const
274+
{
275+
// Three cases here in which we should not read the collection:
276+
// 1. The collection has zero or a negative number of elements or
277+
// 2. The length in bytes of the collection is zero or negative
278+
// 3. The remaining part of the buffer is too short to read the collection
279+
return nElements <= 0 || lengthInBytes <= 0 || (size_t)lengthInBytes > (size_t)(fBufMax - fBufCur);
280+
}
281+
271282
//______________________________________________________________________________
272283
inline void TBufferFile::WriteBool(Bool_t b)
273284
{

‎io/io/src/TBufferFile.cxx‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -737,7 +737,7 @@ Int_t TBufferFile::ReadArray(Bool_t *&b)
737737
Int_t n;
738738
*this >> n;
739739

740-
if (n <= 0 || n > fBufSize) return 0;
740+
if (ShouldNotReadCollection(n)) return 0;
741741

742742
if (!b) b = new Bool_t[n];
743743

@@ -766,7 +766,7 @@ Int_t TBufferFile::ReadArray(Char_t *&c)
766766
*this >> n;
767767
Int_t l = sizeof(Char_t)*n;
768768

769-
if (l <= 0 || l > fBufSize) return 0;
769+
if (ShouldNotReadCollection(l, n)) return 0;
770770

771771
if (!c) c = new Char_t[n];
772772

@@ -789,7 +789,7 @@ Int_t TBufferFile::ReadArray(Short_t *&h)
789789
*this >> n;
790790
Int_t l = sizeof(Short_t)*n;
791791

792-
if (l <= 0 || l > fBufSize) return 0;
792+
if (ShouldNotReadCollection(l, n)) return 0;
793793

794794
if (!h) h = new Short_t[n];
795795

@@ -822,7 +822,7 @@ Int_t TBufferFile::ReadArray(Int_t *&ii)
822822
*this >> n;
823823
Int_t l = sizeof(Int_t)*n;
824824

825-
if (l <= 0 || l > fBufSize) return 0;
825+
if (ShouldNotReadCollection(l, n)) return 0;
826826

827827
if (!ii) ii = new Int_t[n];
828828

@@ -855,7 +855,7 @@ Int_t TBufferFile::ReadArray(Long_t *&ll)
855855
*this >> n;
856856
Int_t l = sizeof(Long_t)*n;
857857

858-
if (l <= 0 || l > fBufSize) return 0;
858+
if (ShouldNotReadCollection(l, n)) return 0;
859859

860860
if (!ll) ll = new Long_t[n];
861861

@@ -881,7 +881,7 @@ Int_t TBufferFile::ReadArray(Long64_t *&ll)
881881
*this >> n;
882882
Int_t l = sizeof(Long64_t)*n;
883883

884-
if (l <= 0 || l > fBufSize) return 0;
884+
if (ShouldNotReadCollection(l, n)) return 0;
885885

886886
if (!ll) ll = new Long64_t[n];
887887

@@ -909,7 +909,7 @@ Int_t TBufferFile::ReadArray(Float_t *&f)
909909
*this >> n;
910910
Int_t l = sizeof(Float_t)*n;
911911

912-
if (l <= 0 || l > fBufSize) return 0;
912+
if (ShouldNotReadCollection(l, n)) return 0;
913913

914914
if (!f) f = new Float_t[n];
915915

@@ -942,7 +942,7 @@ Int_t TBufferFile::ReadArray(Double_t *&d)
942942
*this >> n;
943943
Int_t l = sizeof(Double_t)*n;
944944

945-
if (l <= 0 || l > fBufSize) return 0;
945+
if (ShouldNotReadCollection(l, n)) return 0;
946946

947947
if (!d) d = new Double_t[n];
948948

@@ -970,7 +970,7 @@ Int_t TBufferFile::ReadArrayFloat16(Float_t *&f, TStreamerElement *ele)
970970
Int_t n;
971971
*this >> n;
972972

973-
if (n <= 0 || 3*n > fBufSize) return 0;
973+
if (ShouldNotReadCollection(3*n, n)) return 0;
974974

975975
if (!f) f = new Float_t[n];
976976

@@ -992,7 +992,7 @@ Int_t TBufferFile::ReadArrayDouble32(Double_t *&d, TStreamerElement *ele)
992992
Int_t n;
993993
*this >> n;
994994

995-
if (n <= 0 || 3*n > fBufSize) return 0;
995+
if (ShouldNotReadCollection(3*n, n)) return 0;
996996

997997
if (!d) d = new Double_t[n];
998998

@@ -1012,7 +1012,7 @@ Int_t TBufferFile::ReadStaticArray(Bool_t *b)
10121012
Int_t n;
10131013
*this >> n;
10141014

1015-
if (n <= 0 || n > fBufSize) return 0;
1015+
if (ShouldNotReadCollection(n)) return 0;
10161016

10171017
if (!b) return 0;
10181018

@@ -1040,7 +1040,7 @@ Int_t TBufferFile::ReadStaticArray(Char_t *c)
10401040
*this >> n;
10411041
Int_t l = sizeof(Char_t)*n;
10421042

1043-
if (l <= 0 || l > fBufSize) return 0;
1043+
if (ShouldNotReadCollection(l, n)) return 0;
10441044

10451045
if (!c) return 0;
10461046

@@ -1062,7 +1062,7 @@ Int_t TBufferFile::ReadStaticArray(Short_t *h)
10621062
*this >> n;
10631063
Int_t l = sizeof(Short_t)*n;
10641064

1065-
if (l <= 0 || l > fBufSize) return 0;
1065+
if (ShouldNotReadCollection(l, n)) return 0;
10661066

10671067
if (!h) return 0;
10681068

@@ -1094,7 +1094,7 @@ Int_t TBufferFile::ReadStaticArray(Int_t *ii)
10941094
*this >> n;
10951095
Int_t l = sizeof(Int_t)*n;
10961096

1097-
if (l <= 0 || l > fBufSize) return 0;
1097+
if (ShouldNotReadCollection(l, n)) return 0;
10981098

10991099
if (!ii) return 0;
11001100

@@ -1126,7 +1126,7 @@ Int_t TBufferFile::ReadStaticArray(Long_t *ll)
11261126
*this >> n;
11271127
Int_t l = sizeof(Long_t)*n;
11281128

1129-
if (l <= 0 || l > fBufSize) return 0;
1129+
if (ShouldNotReadCollection(l, n)) return 0;
11301130

11311131
if (!ll) return 0;
11321132

@@ -1151,7 +1151,7 @@ Int_t TBufferFile::ReadStaticArray(Long64_t *ll)
11511151
*this >> n;
11521152
Int_t l = sizeof(Long64_t)*n;
11531153

1154-
if (l <= 0 || l > fBufSize) return 0;
1154+
if (ShouldNotReadCollection(l, n)) return 0;
11551155

11561156
if (!ll) return 0;
11571157

@@ -1178,7 +1178,7 @@ Int_t TBufferFile::ReadStaticArray(Float_t *f)
11781178
*this >> n;
11791179
Int_t l = sizeof(Float_t)*n;
11801180

1181-
if (n <= 0 || l > fBufSize) return 0;
1181+
if (ShouldNotReadCollection(l, n)) return 0;
11821182

11831183
if (!f) return 0;
11841184

@@ -1210,7 +1210,7 @@ Int_t TBufferFile::ReadStaticArray(Double_t *d)
12101210
*this >> n;
12111211
Int_t l = sizeof(Double_t)*n;
12121212

1213-
if (n <= 0 || l > fBufSize) return 0;
1213+
if (ShouldNotReadCollection(l, n)) return 0;
12141214

12151215
if (!d) return 0;
12161216

@@ -1237,7 +1237,7 @@ Int_t TBufferFile::ReadStaticArrayFloat16(Float_t *f, TStreamerElement *ele)
12371237
Int_t n;
12381238
*this >> n;
12391239

1240-
if (n <= 0 || 3*n > fBufSize) return 0;
1240+
if (ShouldNotReadCollection(3*n, n)) return 0;
12411241

12421242
if (!f) return 0;
12431243

@@ -1258,7 +1258,7 @@ Int_t TBufferFile::ReadStaticArrayDouble32(Double_t *d, TStreamerElement *ele)
12581258
Int_t n;
12591259
*this >> n;
12601260

1261-
if (n <= 0 || 3*n > fBufSize) return 0;
1261+
if (ShouldNotReadCollection(3*n, n)) return 0;
12621262

12631263
if (!d) return 0;
12641264

@@ -1272,7 +1272,7 @@ Int_t TBufferFile::ReadStaticArrayDouble32(Double_t *d, TStreamerElement *ele)
12721272

12731273
void TBufferFile::ReadFastArray(Bool_t *b, Int_t n)
12741274
{
1275-
if (n <= 0 || n > fBufSize) return;
1275+
if (ShouldNotReadCollection(n)) return;
12761276

12771277
if (sizeof(Bool_t) > 1) {
12781278
for (int i = 0; i < n; i++)
@@ -1289,7 +1289,7 @@ void TBufferFile::ReadFastArray(Bool_t *b, Int_t n)
12891289

12901290
void TBufferFile::ReadFastArray(Char_t *c, Int_t n)
12911291
{
1292-
if (n <= 0 || n > fBufSize) return;
1292+
if (ShouldNotReadCollection(n)) return;
12931293

12941294
Int_t l = sizeof(Char_t)*n;
12951295
memcpy(c, fBufCur, l);
@@ -1310,7 +1310,7 @@ void TBufferFile::ReadFastArrayString(Char_t *c, Int_t n)
13101310
*this >> len;
13111311
}
13121312
if (len) {
1313-
if (len <= 0 || len > fBufSize) return;
1313+
if (ShouldNotReadCollection(len)) return;
13141314
Int_t blen = len;
13151315
if (len >= n) len = n-1;
13161316

@@ -1330,7 +1330,7 @@ void TBufferFile::ReadFastArrayString(Char_t *c, Int_t n)
13301330
void TBufferFile::ReadFastArray(Short_t *h, Int_t n)
13311331
{
13321332
Int_t l = sizeof(Short_t)*n;
1333-
if (n <= 0 || l > fBufSize) return;
1333+
if (ShouldNotReadCollection(l, n)) return;
13341334

13351335
#ifdef R__BYTESWAP
13361336
# ifdef USE_BSWAPCPY
@@ -1352,7 +1352,7 @@ void TBufferFile::ReadFastArray(Short_t *h, Int_t n)
13521352
void TBufferFile::ReadFastArray(Int_t *ii, Int_t n)
13531353
{
13541354
Int_t l = sizeof(Int_t)*n;
1355-
if (l <= 0 || l > fBufSize) return;
1355+
if (ShouldNotReadCollection(l, n)) return;
13561356

13571357
#ifdef R__BYTESWAP
13581358
# ifdef USE_BSWAPCPY
@@ -1374,7 +1374,7 @@ void TBufferFile::ReadFastArray(Int_t *ii, Int_t n)
13741374
void TBufferFile::ReadFastArray(Long_t *ll, Int_t n)
13751375
{
13761376
Int_t l = sizeof(Long_t)*n;
1377-
if (l <= 0 || l > fBufSize) return;
1377+
if (ShouldNotReadCollection(l, n)) return;
13781378

13791379
TFile *file = (TFile*)fParent;
13801380
if (file && file->GetVersion() < 30006) {
@@ -1390,7 +1390,7 @@ void TBufferFile::ReadFastArray(Long_t *ll, Int_t n)
13901390
void TBufferFile::ReadFastArray(Long64_t *ll, Int_t n)
13911391
{
13921392
Int_t l = sizeof(Long64_t)*n;
1393-
if (l <= 0 || l > fBufSize) return;
1393+
if (ShouldNotReadCollection(l, n)) return;
13941394

13951395
#ifdef R__BYTESWAP
13961396
for (int i = 0; i < n; i++)
@@ -1407,7 +1407,7 @@ void TBufferFile::ReadFastArray(Long64_t *ll, Int_t n)
14071407
void TBufferFile::ReadFastArray(Float_t *f, Int_t n)
14081408
{
14091409
Int_t l = sizeof(Float_t)*n;
1410-
if (l <= 0 || l > fBufSize) return;
1410+
if (ShouldNotReadCollection(l, n)) return;
14111411

14121412
#ifdef R__BYTESWAP
14131413
# ifdef USE_BSWAPCPY
@@ -1429,7 +1429,7 @@ void TBufferFile::ReadFastArray(Float_t *f, Int_t n)
14291429
void TBufferFile::ReadFastArray(Double_t *d, Int_t n)
14301430
{
14311431
Int_t l = sizeof(Double_t)*n;
1432-
if (l <= 0 || l > fBufSize) return;
1432+
if (ShouldNotReadCollection(l, n)) return;
14331433

14341434
#ifdef R__BYTESWAP
14351435
for (int i = 0; i < n; i++)
@@ -1446,7 +1446,7 @@ void TBufferFile::ReadFastArray(Double_t *d, Int_t n)
14461446

14471447
void TBufferFile::ReadFastArrayFloat16(Float_t *f, Int_t n, TStreamerElement *elem)
14481448
{
1449-
if (n <= 0 || 3*n > fBufSize) return;
1449+
if (ShouldNotReadCollection(3*n, n)) return;
14501450

14511451
// The parameter should be const, however we have not yet decided how to
14521452
// transition the signature since the function is virtual. This ensures
@@ -1492,7 +1492,7 @@ void TBufferFile::ReadFastArrayFloat16(Float_t *f, Int_t n, TStreamerElement *el
14921492

14931493
void TBufferFile::ReadFastArrayWithFactor(Float_t *ptr, Int_t n, Double_t factor, Double_t minvalue)
14941494
{
1495-
if (n <= 0 || 3*n > fBufSize) return;
1495+
if (ShouldNotReadCollection(3*n, n)) return;
14961496

14971497
//a range was specified. We read an integer and convert it back to a float
14981498
for (int j=0;j < n; j++) {
@@ -1506,7 +1506,7 @@ void TBufferFile::ReadFastArrayWithFactor(Float_t *ptr, Int_t n, Double_t factor
15061506

15071507
void TBufferFile::ReadFastArrayWithNbits(Float_t *ptr, Int_t n, Int_t nbits)
15081508
{
1509-
if (n <= 0 || 3*n > fBufSize) return;
1509+
if (ShouldNotReadCollection(3*n, n)) return;
15101510

15111511
if (!nbits) nbits = 12;
15121512
//we read the exponent and the truncated mantissa of the float
@@ -1539,7 +1539,7 @@ void TBufferFile::ReadFastArrayDouble32(Double_t *d, Int_t n, TStreamerElement *
15391539
// that the function does not inadvertently use non-const parts of
15401540
// TStreamerElement.
15411541
const TStreamerElement *ele = elem;
1542-
if (n <= 0 || 3*n > fBufSize) return;
1542+
if (ShouldNotReadCollection(3*n, n)) return;
15431543

15441544
if (ele && ele->GetFactor() != 0) {
15451545
//a range was specified. We read an integer and convert it back to a double.
@@ -1587,7 +1587,7 @@ void TBufferFile::ReadFastArrayDouble32(Double_t *d, Int_t n, TStreamerElement *
15871587

15881588
void TBufferFile::ReadFastArrayWithFactor(Double_t *d, Int_t n, Double_t factor, Double_t minvalue)
15891589
{
1590-
if (n <= 0 || 3*n > fBufSize) return;
1590+
if (ShouldNotReadCollection(3*n, n)) return;
15911591

15921592
//a range was specified. We read an integer and convert it back to a double.
15931593
for (int j=0;j < n; j++) {
@@ -1601,7 +1601,7 @@ void TBufferFile::ReadFastArrayWithFactor(Double_t *d, Int_t n, Double_t factor,
16011601

16021602
void TBufferFile::ReadFastArrayWithNbits(Double_t *d, Int_t n, Int_t nbits)
16031603
{
1604-
if (n <= 0 || 3*n > fBufSize) return;
1604+
if (ShouldNotReadCollection(3*n, n)) return;
16051605

16061606
if (!nbits) {
16071607
//we read a float and convert it to double

0 commit comments

Comments
 (0)