Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions tree/ntuple/src/RMiniFile.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -823,19 +823,26 @@ ROOT::RResult<ROOT::RNTuple> ROOT::Internal::RMiniFileReader::GetNTupleProperAtO
std::uint64_t compSize,
std::uint64_t uncompLen)
{
if (compSize > uncompLen) {
return R__FAIL("Invalid size and length combination of anchor at offset " + std::to_string(payloadOffset) +
" (length: " + std::to_string(uncompLen) + ", bytes on disk: " + std::to_string(compSize) + ")");
}

Comment thread
jblomer marked this conversation as resolved.
Resolved
// The object length can be smaller than the size of RTFNTuple if it comes from a past RNTuple class version,
// or larger than it if it comes from a future RNTuple class version.
auto bufAnchor = MakeUninitArray<unsigned char>(std::max<size_t>(uncompLen, sizeof(RTFNTuple)));
RTFNTuple *ntuple = new (bufAnchor.get()) RTFNTuple;

if (compSize != uncompLen) {
if (compSize < uncompLen) {
// Read into a temporary buffer
auto unzipBuf = MakeUninitArray<unsigned char>(std::max<size_t>(uncompLen, sizeof(RTFNTuple)));
ReadBuffer(unzipBuf.get(), compSize, payloadOffset);
auto zipBuf = MakeUninitArray<unsigned char>(compSize);
ReadBuffer(zipBuf.get(), compSize, payloadOffset);
// Unzip into the final buffer
RNTupleDecompressor::Unzip(unzipBuf.get(), compSize, uncompLen, ntuple);
RNTupleDecompressor::Unzip(zipBuf.get(), compSize, uncompLen, ntuple);
} else {
ReadBuffer(ntuple, compSize, payloadOffset);
// We can directly decompress into ntuple because this RTFNTuple object has been constructed in a memory area
// of (max(uncompLen, sizeof(RTFNTuple))) bytes.
ReadBuffer(ntuple, uncompLen, payloadOffset);
}

// We require that future class versions only append members and store the checksum in the last 8 bytes
Expand Down
47 changes: 47 additions & 0 deletions tree/ntuple/test/ntuple_minifile.cxx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "ntuple_test.hxx"
#include <Byteswap.h>
#include <TKey.h>
#include <TTree.h>
#include <TUUID.h>
Expand Down Expand Up @@ -932,3 +933,49 @@ TEST(MiniFile, FreeSlots)
EXPECT_TRUE(found);
}
}

TEST(MiniFile, InvalidAnchor)
{
FileRaii fileGuard("test_ntuple_minifile_invalid_anchor.root");
RNTupleWriteOptions options;
auto writer = RNTupleFileWriter::Recreate("ntpl", fileGuard.GetPath(), EContainerFormat::kTFile, options);
writer->Commit();

auto tfile = std::unique_ptr<TFile>(TFile::Open(fileGuard.GetPath().c_str()));
ASSERT_TRUE(tfile && !tfile->IsZombie());
Long64_t seekKey = 0;
int objLen = 0;
int nbytes = 0;
auto keys = tfile->WalkTKeys();
for (const auto &k : keys) {
if (k.fKeyName == "ntpl") {
seekKey = k.fSeekKey;
objLen = k.fObjLen;
ASSERT_GT(k.fLen, k.fKeyLen);
nbytes = k.fLen - k.fKeyLen;
break;
}
}
ASSERT_GT(seekKey, 0);
ASSERT_GE(objLen, nbytes);

// We corrupt the RNTuple anchor key such that the size on disk becomes _larger_ than the length of the object.
auto fp = fopen(fileGuard.GetPath().c_str(), "r+b");
Comment thread
jblomer marked this conversation as resolved.
Resolved
Comment thread
jblomer marked this conversation as resolved.
Resolved
ASSERT_TRUE(fp != nullptr);
ASSERT_EQ(0, fseek(fp, seekKey, SEEK_SET));
std::int32_t oversizedLen = objLen - nbytes + 1;
#ifdef R__BYTESWAP
std::int32_t onDiskVal = RByteSwap<4>::bswap(oversizedLen);
ASSERT_EQ(1u, fwrite(&onDiskVal, sizeof(std::int32_t), 1, fp));
#else
ASSERT_EQ(1u, fwrite(&oversizedLen, sizeof(std::int32_t), 1, fp));
#endif
fclose(fp);

try {
RNTupleReader::Open("ntpl", fileGuard.GetPath());
FAIL() << "invalid anchor key should fail";
} catch (const ROOT::RException &e) {
EXPECT_THAT(e.what(), testing::HasSubstr("Invalid size and length combination of anchor"));
}
}
Loading