Bug #18094
tca::PrimaryID() can access invalid memory
100%
Description
Both functions tca::PrimaryID() in larreco:source:larreco/RecoAlg/TCAlg/Utils.cxx look for a parent ID recursively.
They lack the check at each iteration of whether the next ID in the recursion is valid or not.
It may in fact happen that the ID is 0, but the algorithm requires it to be at least 1, e.g.:
auto& tj = tjs.allTraj[parid - 1];This causes an invalid read to the element
-1 of the array, which apparently does not necessarily cause a critical failure.
It is not clear to me what the correct behaviour of the function should be (maybe return -1?).
History
#1 Updated by Gianluca Petrillo about 3 years ago
- Status changed from New to Assigned
- Assignee set to Bruce Baller
#2 Updated by Tingjun Yang about 3 years ago
This is fixed by Bruce's commit larreco:01a62ced7e83b8d220a119183dcd339b1f941457.
#3 Updated by Gianluca Petrillo about 3 years ago
Do we really want larreco:source:larreco/RecoAlg/TCAlg/Utils.cxx@01a62ce#L469?
std::cout<<"";#4 Updated by Tingjun Yang about 3 years ago
Gianluca Petrillo wrote:
Do we really want larreco:source:larreco/RecoAlg/TCAlg/Utils.cxx@01a62ce#L469?
[...]
There are other changes in that commit:
https://cdcvs.fnal.gov/redmine/projects/larreco/repository/revisions/01a62ced7e83b8d220a119183dcd339b1f941457/diff/larreco/RecoAlg/TCAlg/Utils.cxx
#5 Updated by Gianluca Petrillo about 3 years ago
Sure. I was questioning that specific line, not the rest of the commit.
#6 Updated by Tingjun Yang about 3 years ago
Gianluca Petrillo wrote:
Sure. I was questioning that specific line, not the rest of the commit.
Looks like a line that does not do anything. Doesn't do any harm either.
#8 Updated by Bruce Baller over 2 years ago
- Status changed from Assigned to Closed