Skip to content

Commit 6a53a6e

Browse files
authored
fix: validate forest in Mmr::open_at to prevent OOB access (#553)
1 parent 262858e commit 6a53a6e

File tree

2 files changed

+17
-9
lines changed

2 files changed

+17
-9
lines changed

miden-crypto/src/merkle/mmr/error.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ pub enum MmrError {
1010
PositionNotFound(usize),
1111
#[error("mmr peaks are invalid: {0}")]
1212
InvalidPeaks(String),
13+
#[error("mmr forest is out of bounds: requested {0} > current {1}")]
14+
ForestOutOfBounds(usize, usize),
1315
#[error("mmr peak does not match the computed merkle root of the provided authentication path")]
1416
PeakPathMismatch,
1517
#[error("requested peak index is {peak_idx} but the number of peaks is {peaks_len}")]

miden-crypto/src/merkle/mmr/full.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ impl Mmr {
9797
/// - The specified leaf position is out of bounds for this MMR.
9898
/// - The specified `forest` value is not valid for this MMR.
9999
pub fn open_at(&self, pos: usize, forest: Forest) -> Result<MmrProof, MmrError> {
100+
if forest > self.forest {
101+
return Err(MmrError::ForestOutOfBounds(forest.num_leaves(), self.forest.num_leaves()));
102+
}
100103
let (_, path) = self.collect_merkle_path_and_value(pos, forest)?;
101104

102105
Ok(MmrProof {
@@ -150,10 +153,7 @@ impl Mmr {
150153
/// Returns an error if the specified `forest` value is not valid for this MMR.
151154
pub fn peaks_at(&self, forest: Forest) -> Result<MmrPeaks, MmrError> {
152155
if forest > self.forest {
153-
return Err(MmrError::InvalidPeaks(format!(
154-
"requested forest {forest} exceeds current forest {}",
155-
self.forest
156-
)));
156+
return Err(MmrError::ForestOutOfBounds(forest.num_leaves(), self.forest.num_leaves()));
157157
}
158158

159159
let peaks: Vec<Word> = TreeSizeIterator::new(forest)
@@ -177,11 +177,17 @@ impl Mmr {
177177
/// The result is a packed sequence of the authentication elements required to update the trees
178178
/// that have been merged together, followed by the new peaks of the [Mmr].
179179
pub fn get_delta(&self, from_forest: Forest, to_forest: Forest) -> Result<MmrDelta, MmrError> {
180-
if to_forest > self.forest || from_forest > to_forest {
181-
return Err(MmrError::InvalidPeaks(format!(
182-
"to_forest {to_forest} exceeds the current forest {} or from_forest {from_forest} exceeds to_forest",
183-
self.forest
184-
)));
180+
if to_forest > self.forest {
181+
return Err(MmrError::ForestOutOfBounds(
182+
to_forest.num_leaves(),
183+
self.forest.num_leaves(),
184+
));
185+
}
186+
if from_forest > to_forest {
187+
return Err(MmrError::ForestOutOfBounds(
188+
from_forest.num_leaves(),
189+
to_forest.num_leaves(),
190+
));
185191
}
186192

187193
if from_forest == to_forest {

0 commit comments

Comments
 (0)