Skip to content

Commit 6c4f869

Browse files
romanzacvinhtc27
andauthored
test: Cover code with potential to overflow panic (#382)
## Description A batch of tests to cover places in code which may be prone to overflow or panic. ## Tests modified/added - test_pmtree_depth_shift_overflow - test_pmtree_override_range_min_index_underflow - test_bytes_le_to_rln_proof_short - test_bytes_be_to_rln_proof_short - test_bytes_le_to_rln_proof_empty - test_bytes_be_to_rln_proof_empty - test_rln_witness_to_bigint_json_fields - test_length_prefix_overflow - test_poseidon_grain_lfsr_new_panics_on_invalid_is_field - test_poseidon_grain_lfsr_new_panics_on_invalid_is_sbox_an_inverse - test_find_poseidon_ark_and_mds_bn254_regression_no_inverse_panic - test_full_merkle_tree_new_depth_shift_overflow - test_optimal_merkle_tree_new_depth_shift_overflow - test_full_merkle_tree_set_range_start_overflow - test_optimal_merkle_tree_set_range_start_overflow - test_full_merkle_tree_override_range_min_index_underflow - test_optimal_merkle_tree_override_range_min_index_underflow ## Issues reported - #383 - #384 ## Coverage changed Before 88.98% [Download HTML Report](https://github.com/vacp2p/zerokit/actions/runs/22339493470/artifacts/5630265525) After 88.12% [Download HTML Report](https://github.com/vacp2p/zerokit/actions/runs/23179431489/artifacts/5958379649) ## Checklist - [x] My PR title follows [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) format - [x] I have linked the related issue(s) - [x] `cargo fmt --all -- --check` produces no changes - [x] Clippy passes for all affected crate/feature combinations (see [Linting](#linting-mirrors-ci) above) - [x] `make test` passes locally - [x] No new `unwrap()` / `expect()` / `panic!()` in library code - [x] New code includes appropriate tests (unit / integration / WASM where applicable) - [x] I have run the CI coverage report — add the `run-coverage` label to enable it - [x] All CI checks pass and the PR is marked **Ready for review** --------- Co-authored-by: vinhtc27 <vinhtc27@gmail.com>
1 parent 25d398b commit 6c4f869

File tree

10 files changed

+177
-18
lines changed

10 files changed

+177
-18
lines changed

rln/src/pm_tree_adapter.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use zerokit_utils::{
1111
},
1212
pm_tree::{
1313
pmtree,
14-
pmtree::{tree::Key, Database, Hasher, PmtreeErrorKind},
14+
pmtree::{tree::Key, Database, Hasher, PmtreeErrorKind, TreeErrorKind},
1515
Config, Mode, SledDB,
1616
},
1717
};
@@ -239,9 +239,15 @@ impl ZerokitMerkleTree for PmTree {
239239
Err(_) => pmtree::MerkleTree::new(depth, config.0)?,
240240
};
241241

242+
let capacity = 1usize.checked_shl(depth as u32).ok_or({
243+
ZerokitMerkleTreeError::PmtreeErrorKind(PmtreeErrorKind::TreeError(
244+
TreeErrorKind::IndexOutOfBounds,
245+
))
246+
})?;
247+
242248
Ok(PmTree {
243249
tree,
244-
cached_leaves_indices: vec![0; 1 << depth],
250+
cached_leaves_indices: vec![0; capacity],
245251
metadata: Vec::new(),
246252
})
247253
}

rln/src/utils.rs

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,9 @@ pub fn bytes_le_to_vec_u8(input: &[u8]) -> Result<(Vec<u8>, usize), UtilsError>
244244
}
245245
let len = usize::try_from(u64::from_le_bytes(input[0..VEC_LEN_BYTE_SIZE].try_into()?))?;
246246
read += VEC_LEN_BYTE_SIZE;
247-
if input.len() < VEC_LEN_BYTE_SIZE + len {
247+
if len > input.len() - VEC_LEN_BYTE_SIZE {
248248
return Err(UtilsError::InsufficientData {
249-
expected: VEC_LEN_BYTE_SIZE + len,
249+
expected: VEC_LEN_BYTE_SIZE.saturating_add(len),
250250
actual: input.len(),
251251
});
252252
}
@@ -266,9 +266,9 @@ pub fn bytes_be_to_vec_u8(input: &[u8]) -> Result<(Vec<u8>, usize), UtilsError>
266266
}
267267
let len = usize::try_from(u64::from_be_bytes(input[0..VEC_LEN_BYTE_SIZE].try_into()?))?;
268268
read += VEC_LEN_BYTE_SIZE;
269-
if input.len() < VEC_LEN_BYTE_SIZE + len {
269+
if len > input.len() - VEC_LEN_BYTE_SIZE {
270270
return Err(UtilsError::InsufficientData {
271-
expected: VEC_LEN_BYTE_SIZE + len,
271+
expected: VEC_LEN_BYTE_SIZE.saturating_add(len),
272272
actual: input.len(),
273273
});
274274
}
@@ -289,9 +289,9 @@ pub fn bytes_le_to_vec_fr(input: &[u8]) -> Result<(Vec<Fr>, usize), UtilsError>
289289
let len = usize::try_from(u64::from_le_bytes(input[0..VEC_LEN_BYTE_SIZE].try_into()?))?;
290290
read += VEC_LEN_BYTE_SIZE;
291291
let el_size = FR_BYTE_SIZE;
292-
if input.len() < VEC_LEN_BYTE_SIZE + len * el_size {
292+
if len > (input.len() - VEC_LEN_BYTE_SIZE) / el_size {
293293
return Err(UtilsError::InsufficientData {
294-
expected: VEC_LEN_BYTE_SIZE + len * el_size,
294+
expected: VEC_LEN_BYTE_SIZE.saturating_add(len.saturating_mul(el_size)),
295295
actual: input.len(),
296296
});
297297
}
@@ -318,9 +318,9 @@ pub fn bytes_be_to_vec_fr(input: &[u8]) -> Result<(Vec<Fr>, usize), UtilsError>
318318
let len = usize::try_from(u64::from_be_bytes(input[0..VEC_LEN_BYTE_SIZE].try_into()?))?;
319319
read += VEC_LEN_BYTE_SIZE;
320320
let el_size = FR_BYTE_SIZE;
321-
if input.len() < VEC_LEN_BYTE_SIZE + len * el_size {
321+
if len > (input.len() - VEC_LEN_BYTE_SIZE) / el_size {
322322
return Err(UtilsError::InsufficientData {
323-
expected: VEC_LEN_BYTE_SIZE + len * el_size,
323+
expected: VEC_LEN_BYTE_SIZE.saturating_add(len.saturating_mul(el_size)),
324324
actual: input.len(),
325325
});
326326
}
@@ -347,9 +347,10 @@ pub fn bytes_le_to_vec_usize(input: &[u8]) -> Result<Vec<usize>, UtilsError> {
347347
if nof_elem == 0 {
348348
Ok(vec![])
349349
} else {
350-
if input.len() < VEC_LEN_BYTE_SIZE + nof_elem * VEC_LEN_BYTE_SIZE {
350+
if nof_elem > (input.len() - VEC_LEN_BYTE_SIZE) / VEC_LEN_BYTE_SIZE {
351351
return Err(UtilsError::InsufficientData {
352-
expected: VEC_LEN_BYTE_SIZE + nof_elem * VEC_LEN_BYTE_SIZE,
352+
expected: VEC_LEN_BYTE_SIZE
353+
.saturating_add(nof_elem.saturating_mul(VEC_LEN_BYTE_SIZE)),
353354
actual: input.len(),
354355
});
355356
}
@@ -377,9 +378,10 @@ pub fn bytes_be_to_vec_usize(input: &[u8]) -> Result<Vec<usize>, UtilsError> {
377378
if nof_elem == 0 {
378379
Ok(vec![])
379380
} else {
380-
if input.len() < VEC_LEN_BYTE_SIZE + nof_elem * VEC_LEN_BYTE_SIZE {
381+
if nof_elem > (input.len() - VEC_LEN_BYTE_SIZE) / VEC_LEN_BYTE_SIZE {
381382
return Err(UtilsError::InsufficientData {
382-
expected: VEC_LEN_BYTE_SIZE + nof_elem * VEC_LEN_BYTE_SIZE,
383+
expected: VEC_LEN_BYTE_SIZE
384+
.saturating_add(nof_elem.saturating_mul(VEC_LEN_BYTE_SIZE)),
383385
actual: input.len(),
384386
});
385387
}
@@ -406,9 +408,9 @@ pub fn bytes_le_to_vec_bool(input: &[u8]) -> Result<(Vec<bool>, usize), UtilsErr
406408
}
407409
let len = usize::try_from(u64::from_le_bytes(input[0..VEC_LEN_BYTE_SIZE].try_into()?))?;
408410
read += VEC_LEN_BYTE_SIZE;
409-
if input.len() < VEC_LEN_BYTE_SIZE + len {
411+
if len > input.len() - VEC_LEN_BYTE_SIZE {
410412
return Err(UtilsError::InsufficientData {
411-
expected: VEC_LEN_BYTE_SIZE + len,
413+
expected: VEC_LEN_BYTE_SIZE.saturating_add(len),
412414
actual: input.len(),
413415
});
414416
}
@@ -431,9 +433,9 @@ pub fn bytes_be_to_vec_bool(input: &[u8]) -> Result<(Vec<bool>, usize), UtilsErr
431433
}
432434
let len = usize::try_from(u64::from_be_bytes(input[0..VEC_LEN_BYTE_SIZE].try_into()?))?;
433435
read += VEC_LEN_BYTE_SIZE;
434-
if input.len() < VEC_LEN_BYTE_SIZE + len {
436+
if len > input.len() - VEC_LEN_BYTE_SIZE {
435437
return Err(UtilsError::InsufficientData {
436-
expected: VEC_LEN_BYTE_SIZE + len,
438+
expected: VEC_LEN_BYTE_SIZE.saturating_add(len),
437439
actual: input.len(),
438440
});
439441
}

rln/tests/pm_tree.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,27 @@ mod test {
139139
));
140140
}
141141

142+
#[test]
143+
fn test_pmtree_depth_shift_overflow() {
144+
let depth = usize::BITS as usize;
145+
let result = PmTree::new(depth, Fr::zero(), temp_config());
146+
assert!(matches!(
147+
result,
148+
Err(ZerokitMerkleTreeError::PmtreeErrorKind(_))
149+
));
150+
}
151+
152+
#[test]
153+
fn test_pmtree_override_range_min_index_underflow() {
154+
let mut tree = PmTree::new(TEST_DEPTH, Fr::zero(), temp_config()).unwrap();
155+
let result =
156+
tree.override_range(0, vec![Fr::from(1)].into_iter(), vec![5usize].into_iter());
157+
assert!(matches!(
158+
result,
159+
Err(ZerokitMerkleTreeError::InvalidIndices)
160+
));
161+
}
162+
142163
#[test]
143164
fn test_pmtree_basic_operations() {
144165
let mut tree = PmTree::default(TEST_DEPTH).unwrap();

rln/tests/protocol.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,30 @@ mod test {
739739
assert_eq!(computed_root2, root2);
740740
}
741741

742+
#[test]
743+
fn test_bytes_le_to_rln_proof_short() {
744+
let bytes = vec![0u8; COMPRESS_PROOF_SIZE - 1];
745+
assert!(bytes_le_to_rln_proof(&bytes).is_err());
746+
}
747+
748+
#[test]
749+
fn test_bytes_be_to_rln_proof_short() {
750+
let bytes = vec![0u8; COMPRESS_PROOF_SIZE - 1];
751+
assert!(bytes_be_to_rln_proof(&bytes).is_err());
752+
}
753+
754+
#[test]
755+
fn test_bytes_le_to_rln_proof_empty() {
756+
let bytes = vec![];
757+
assert!(bytes_le_to_rln_proof(&bytes).is_err());
758+
}
759+
760+
#[test]
761+
fn test_bytes_be_to_rln_proof_empty() {
762+
let bytes = vec![];
763+
assert!(bytes_be_to_rln_proof(&bytes).is_err());
764+
}
765+
742766
#[test]
743767
fn test_rln_witness_to_bigint_json_fields() {
744768
// Test with default witness

rln/tests/utils.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,25 @@ mod test {
414414
assert!(bytes_be_to_vec_u8(&valid_u8_data_be).is_ok());
415415
}
416416

417+
#[test]
418+
fn test_length_prefix_overflow() {
419+
let mut overflow_u8 = vec![0u8; 8];
420+
overflow_u8[..8].copy_from_slice(&normalize_usize_le(usize::MAX));
421+
assert!(bytes_le_to_vec_u8(&overflow_u8).is_err());
422+
423+
let mut overflow_u8_be = vec![0u8; 8];
424+
overflow_u8_be[..8].copy_from_slice(&normalize_usize_be(usize::MAX));
425+
assert!(bytes_be_to_vec_u8(&overflow_u8_be).is_err());
426+
427+
let mut overflow_fr = vec![0u8; 8];
428+
overflow_fr[..8].copy_from_slice(&normalize_usize_le(usize::MAX));
429+
assert!(bytes_le_to_vec_fr(&overflow_fr).is_err());
430+
431+
let mut overflow_fr_be = vec![0u8; 8];
432+
overflow_fr_be[..8].copy_from_slice(&normalize_usize_be(usize::MAX));
433+
assert!(bytes_be_to_vec_fr(&overflow_fr_be).is_err());
434+
}
435+
417436
#[test]
418437
fn test_empty_vectors() {
419438
// Test empty vector serialization/deserialization

utils/src/merkle_tree/error.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ pub enum ZerokitMerkleTreeError {
1515
InvalidSubTreeIndex,
1616
#[error("Start level is != from end level")]
1717
InvalidStartAndEndLevel,
18+
#[error("Tree depth exceeds maximum allowed (must be < {})", usize::BITS)]
19+
InvalidDepth,
1820
#[error("set_range got too many leaves")]
1921
TooManySet,
2022
#[error("Unknown error while computing merkle proof")]

utils/src/merkle_tree/full_merkle_tree.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ where
8484
default_leaf: FrOf<Self::Hasher>,
8585
_config: Self::Config,
8686
) -> Result<Self, ZerokitMerkleTreeError> {
87+
if depth >= usize::BITS as usize {
88+
return Err(ZerokitMerkleTreeError::InvalidDepth);
89+
}
90+
8791
// Compute cache node values, leaf to root
8892
let mut cached_nodes: Vec<H::Fr> = Vec::with_capacity(depth + 1);
8993
cached_nodes.push(default_leaf);

utils/src/merkle_tree/optimal_merkle_tree.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ where
7676
default_leaf: H::Fr,
7777
_config: Self::Config,
7878
) -> Result<Self, ZerokitMerkleTreeError> {
79+
if depth >= usize::BITS as usize {
80+
return Err(ZerokitMerkleTreeError::InvalidDepth);
81+
}
82+
7983
// Compute cache node values, leaf to root
8084
let mut cached_nodes: Vec<H::Fr> = Vec::with_capacity(depth + 1);
8185
cached_nodes.push(default_leaf);

utils/src/poseidon/poseidon_constants.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,3 +259,31 @@ pub fn find_poseidon_ark_and_mds<F: PrimeField>(
259259

260260
(ark, mds)
261261
}
262+
263+
#[cfg(test)]
264+
mod test {
265+
use ark_bn254::Fr;
266+
use num_traits::Zero;
267+
268+
use super::*;
269+
270+
#[test]
271+
fn test_find_poseidon_ark_and_mds_bn254_regression_no_inverse_panic() {
272+
let result = std::panic::catch_unwind(|| {
273+
// Parameters match the hardcoded BN254 Poseidon setup used by current tests.
274+
find_poseidon_ark_and_mds::<Fr>(1, 0, 254, 2, 8, 56, 0)
275+
});
276+
277+
assert!(
278+
result.is_ok(),
279+
"find_poseidon_ark_and_mds unexpectedly panicked (possible MDS inverse invariant break)"
280+
);
281+
282+
let (ark, mds) = result.unwrap();
283+
assert_eq!(ark.len(), (8 + 56) * 2);
284+
assert_eq!(mds.len(), 2);
285+
assert_eq!(mds[0].len(), 2);
286+
assert_eq!(mds[1].len(), 2);
287+
assert_ne!(mds[0][0], Fr::zero());
288+
}
289+
}

utils/tests/merkle_tree.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,55 @@ mod test {
121121
assert_ne!(root_before, root_after);
122122
}
123123

124+
#[test]
125+
fn test_full_merkle_tree_new_depth_shift_overflow() {
126+
let depth = usize::BITS as usize;
127+
let result =
128+
FullMerkleTree::<Keccak256>::new(depth, TestFr([0; 32]), FullMerkleConfig::default());
129+
assert!(result.is_err());
130+
}
131+
132+
#[test]
133+
fn test_optimal_merkle_tree_new_depth_shift_overflow() {
134+
let depth = usize::BITS as usize;
135+
let result = OptimalMerkleTree::<Keccak256>::new(
136+
depth,
137+
TestFr([0; 32]),
138+
OptimalMerkleConfig::default(),
139+
);
140+
assert!(result.is_err());
141+
}
142+
143+
#[test]
144+
fn test_full_merkle_tree_set_range_start_overflow() {
145+
let mut tree_full = default_full_merkle_tree(DEFAULT_DEPTH);
146+
let result = tree_full.set_range(usize::MAX, std::iter::once(TestFr::from(1u32)));
147+
assert!(result.is_err());
148+
}
149+
150+
#[test]
151+
fn test_optimal_merkle_tree_set_range_start_overflow() {
152+
let mut tree_opt = default_optimal_merkle_tree(DEFAULT_DEPTH);
153+
let result = tree_opt.set_range(usize::MAX, std::iter::once(TestFr::from(1u32)));
154+
assert!(result.is_err());
155+
}
156+
157+
#[test]
158+
fn test_full_merkle_tree_override_range_min_index_underflow() {
159+
let mut tree_full = default_full_merkle_tree(DEFAULT_DEPTH);
160+
let result =
161+
tree_full.override_range(1, std::iter::once(TestFr::from(1u32)), [5usize].into_iter());
162+
assert!(result.is_err());
163+
}
164+
165+
#[test]
166+
fn test_optimal_merkle_tree_override_range_min_index_underflow() {
167+
let mut tree_opt = default_optimal_merkle_tree(DEFAULT_DEPTH);
168+
let result =
169+
tree_opt.override_range(1, std::iter::once(TestFr::from(1u32)), [5usize].into_iter());
170+
assert!(result.is_err());
171+
}
172+
124173
#[test]
125174
fn test_update_next() {
126175
let mut tree_full = default_full_merkle_tree(DEFAULT_DEPTH);

0 commit comments

Comments
 (0)