Skip to content

Commit a50a9ab

Browse files
authored
fix(merkle): Remove potential overflow in to_scalar_index (#888)
This function previously had the potential to overflow for nodes at depth 64. This commit now checks whether an overflow would occur, and returns an error if this is the case.
1 parent 621dfb2 commit a50a9ab

File tree

3 files changed

+64
-5
lines changed

3 files changed

+64
-5
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
- Added `PublicKey::from_der()` for ECDSA public keys over secp256k1 ([#855](https://github.com/0xMiden/crypto/pull/855)).
2020
- [BREAKING] Removed `RpoRandomCoin` and `RpxRandomCoin` and introduced a Poseidon2-based `RandomCoin` ([#871](https://github.com/0xMiden/crypto/pull/871)).
2121
- Harden MerkleStore deserialization and fuzz coverage ([#878](https://github.com/0xMiden/crypto/pull/878)).
22+
- [BREAKING] Fixed `NodeIndex::to_scalar_index()` overflow at depth 64 by returning `Result<u64, MerkleError>` ([#865](https://github.com/0xMiden/crypto/issues/865)).
2223

2324
## 0.22.4 (2026-03-03)
2425

miden-crypto/src/merkle/index.rs

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,16 @@ impl NodeIndex {
127127
/// Returns the scalar representation of the depth/position pair.
128128
///
129129
/// It is computed as `2^depth + position`.
130-
pub const fn to_scalar_index(&self) -> u64 {
131-
(1 << self.depth as u64) + self.position
130+
///
131+
/// # Errors
132+
///
133+
/// - [`MerkleError::DepthTooBig`] if the depth is 64 or greater, as the resulting index would
134+
/// overflow.
135+
pub const fn to_scalar_index(&self) -> Result<u64, MerkleError> {
136+
if self.depth >= 64 {
137+
return Err(MerkleError::DepthTooBig(self.depth as u64));
138+
}
139+
Ok((1u64 << self.depth as u64) + self.position)
132140
}
133141

134142
/// Returns the depth of the current instance.
@@ -300,5 +308,55 @@ mod tests {
300308
index.move_up();
301309
}
302310
}
311+
312+
#[test]
313+
fn to_scalar_index_succeeds_for_depth_lt_64(depth in 0u8..64, position_bits in 0u64..u64::MAX) {
314+
let position = if depth == 0 { 0 } else { position_bits % (1u64 << depth) };
315+
let index = NodeIndex::new(depth, position).unwrap();
316+
assert!(index.to_scalar_index().is_ok());
317+
}
318+
}
319+
320+
#[test]
321+
fn test_to_scalar_index_depth_64_returns_error() {
322+
let index = NodeIndex::new(64, 0).unwrap();
323+
assert_matches!(index.to_scalar_index(), Err(MerkleError::DepthTooBig(64)));
324+
325+
let index = NodeIndex::new(64, u64::MAX).unwrap();
326+
assert_matches!(index.to_scalar_index(), Err(MerkleError::DepthTooBig(64)));
327+
}
328+
329+
#[test]
330+
fn test_to_scalar_index_known_values() {
331+
// Root's children: depth=1, pos=0 → scalar 2; depth=1, pos=1 → scalar 3
332+
assert_eq!(NodeIndex::make(1, 0).to_scalar_index().unwrap(), 2);
333+
assert_eq!(NodeIndex::make(1, 1).to_scalar_index().unwrap(), 3);
334+
335+
// depth=2: scalars 4,5,6,7
336+
assert_eq!(NodeIndex::make(2, 0).to_scalar_index().unwrap(), 4);
337+
assert_eq!(NodeIndex::make(2, 3).to_scalar_index().unwrap(), 7);
338+
339+
// depth=3: scalars 8..15
340+
assert_eq!(NodeIndex::make(3, 0).to_scalar_index().unwrap(), 8);
341+
assert_eq!(NodeIndex::make(3, 7).to_scalar_index().unwrap(), 15);
342+
}
343+
344+
#[test]
345+
fn test_to_scalar_index_depth_63_max_position() {
346+
// 2^63 + (2^63 - 1) = 2^64 - 1 = u64::MAX
347+
let index = NodeIndex::new(63, (1u64 << 63) - 1).unwrap();
348+
assert_eq!(index.to_scalar_index().unwrap(), u64::MAX);
349+
}
350+
351+
#[test]
352+
fn test_to_scalar_index_boundary_depths() {
353+
// depth 0 (root): scalar = 1 + 0 = 1
354+
assert_eq!(NodeIndex::make(0, 0).to_scalar_index().unwrap(), 1);
355+
356+
// depth 62, position 0: scalar = 2^62
357+
assert_eq!(NodeIndex::make(62, 0).to_scalar_index().unwrap(), 1u64 << 62);
358+
359+
// depth 63, position 0: scalar = 2^63
360+
assert_eq!(NodeIndex::make(63, 0).to_scalar_index().unwrap(), 1u64 << 63);
303361
}
304362
}

miden-crypto/src/merkle/merkle_tree.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ impl MerkleTree {
8888
return Err(MerkleError::DepthTooBig(index.depth() as u64));
8989
}
9090

91-
let pos = index.to_scalar_index() as usize;
91+
let pos = index.to_scalar_index()? as usize;
9292
Ok(self.nodes[pos])
9393
}
9494

@@ -160,13 +160,13 @@ impl MerkleTree {
160160
let pairs: &'a [[Word; 2]] = unsafe { slice::from_raw_parts(ptr, n) };
161161

162162
// update the current node
163-
let pos = index.to_scalar_index() as usize;
163+
let pos = index.to_scalar_index()? as usize;
164164
self.nodes[pos] = value;
165165

166166
// traverse to the root, updating each node with the merged values of its parents
167167
for _ in 0..index.depth() {
168168
index.move_up();
169-
let pos = index.to_scalar_index() as usize;
169+
let pos = index.to_scalar_index()? as usize;
170170
let value = Poseidon2::merge(&pairs[pos]);
171171
self.nodes[pos] = value;
172172
}

0 commit comments

Comments
 (0)