`RNTupleJoinTable::GetEntryIndexes` may read invalid memory

[hageboeck]

Check duplicate issues.

• Checked for duplicates

Description

The following line crashes the test if run with address sanitizer:

root/tree/ntuple/test/ntuple_join_table.cxx

Line 182 in 04a1a9d

auto idx2 = joinTable->GetEntryIndexes({&event, &run});

This happens because internally, argument 0 is interpreted as 16 bit, argument 1 as 64 bit. Given that arguments are read via void * and reinterpret_cast, garbage is read from the stack when the arguments are swapped.

root/tree/ntuple/src/RNTupleJoinTable.cxx

Lines 23 to 28 in 04a1a9d

	switch (fieldValueSize) {
	case 1: value = *reinterpret_cast<std::uint8_t *>(valuePtr); break;
	case 2: value = *reinterpret_cast<std::uint16_t *>(valuePtr); break;
	case 4: value = *reinterpret_cast<std::uint32_t *>(valuePtr); break;
	case 8: value = *reinterpret_cast<std::uint64_t *>(valuePtr); break;
	default: throw ROOT::RException(R__FAIL("value size not supported"));

One could consider:

• Not to include this line in the test (not so great, because this error can easily happen).
• Make GetEntryIndexes accept something like vector<std::uint64_t>
  • The advantage is that the amount of data in the vector is exactly the same as using void *.
  • The disadvantage is that one may have to cast signed integers to unsigned when using the spelling GetEntryIndexes({event, run}) to avoid the narrowing conversion.
• Or make it fully type safe with e.g. a variant.
• Or something completely different like variadic templets etc 🙂

Reproducer

In an asan build, run gtest-tree-ntuple-ntuple-join-table

ROOT version

master

Installation method

Not relevant

Operating system

Not relevant

Additional context

No response