tari-project / tari / 32836793780

77%

Build Type

push

github

Committed by web-flow

Commit Message

fix(mmr): return an error instead of panicking on malformed merkle pr… (#7968)

…oofs

`MerkleProof::verify()` panicked on malformed input rather than
returning an error. `check_root()` only validated the length of the peak
list and then unwrapped the peak hash iterator inside its fold. Any
proof whose sibling path is truncated terminates the walk at a position
that is not a canonical peak, so the fold consumed more items than the
iterator held and unwrapped `None`. A hand-crafted proof with an empty
path and no peaks panicked in the same way. Since these proofs are
fetched from a remote node, a malicious peer could crash the verifying
process.

The fold is now a loop that propagates `IncorrectPeakMap` when the proof
does not carry a hash for every peak, and it also rejects proofs that
carry more peak hashes than were consumed. The boundary check in
`verify_consume()` is tightened to `parent_pos >= self.mmr_size`, since
the largest valid node index is `mmr_size - 1`.

`family()` had the same problem one level down. The type of the literal
in `let peak = 1 << height` was only constrained by the `i128::from()`
below it, which many integer types satisfy, so it defaulted to `i32`. At
height 31 that silently produced `i32::MIN`, corrupting both the peak
mask and the parent arithmetic, and at height 32 and above it panicked
with "attempt to shift left with overflow" - in release builds too,
since the workspace enables overflow checks there. A proof with a long
enough path and a large enough `mmr_size` walks a verification straight
into it, and both of those, along with the leaf index, are supplied by
the remote node in the burn claim flow. The literal is now typed as
`u64`; the smallest node index at height `h` is `2^(h+1) - 2`, so
`height` cannot exceed 63 for a `usize` position and the shift cannot
overflow. The shifts in `family_branch()` and `is_left_sibling()`
correctly infer `usize` from their surrounding arithmetic, but are now
annotated to say so.

... (continued)

Coverage Stats

72 of 73 new or added lines in 4 files covered. (98.63%)

31 existing lines in 10 files now uncovered.

76032 of 119759 relevant lines covered (63.49%)

223825.62 hits per line