Skip to content

fix(slh-dsa): use checked_shl for idx_tree range in tests#1224

Open
sashass1315 wants to merge 2 commits intoRustCrypto:masterfrom
sashass1315:fix/slh-dsa-test-idx-tree-range-256f
Open

fix(slh-dsa): use checked_shl for idx_tree range in tests#1224
sashass1315 wants to merge 2 commits intoRustCrypto:masterfrom
sashass1315:fix/slh-dsa-test-idx-tree-range-256f

Conversation

@sashass1315
Copy link

wrapping_shl(64) on u64 wraps the shift amount modulo 64, producing 1 << 0 = 1 instead of the intended 0. This caused idx_tree range to collapse to 0..=0 for 256f parameter sets (where H - H' = 64), meaning those tests always ran with idx_tree = 0.

Replace wrapping_shl with checked_shl(...).unwrap_or(0) to match the pattern already used in production code [split_digest]

/// Separates the digest into the FORS message, the Xmss tree index, and the Xmss leaf index.
pub(crate) fn split_digest<P: ForsParams>(
digest: &Array<u8, P::M>,
) -> (&Array<u8, P::MD>, u64, u32) {
#[allow(deprecated)]
let m = Array::from_slice(&digest[..P::MD::USIZE]);
let idx_tree_size = (P::H::USIZE - P::HPrime::USIZE).div_ceil(8);
let idx_leaf_size = P::HPrime::USIZE.div_ceil(8);
let mut idx_tree_bytes = [0u8; 8];
let mut idx_leaf_bytes = [0u8; 4];
idx_tree_bytes[8 - idx_tree_size..]
.copy_from_slice(&digest[P::MD::USIZE..P::MD::USIZE + idx_tree_size]);
idx_leaf_bytes[4 - idx_leaf_size..].copy_from_slice(
&digest[P::MD::USIZE + idx_tree_size..P::MD::USIZE + idx_tree_size + idx_leaf_size],
);
// For 256-bit parameters sets, Self::H::U32 - Self::HPrime::U32 = 64
let mask: u64 = 1u64
.checked_shl(P::H::U32 - P::HPrime::U32)
.unwrap_or(0)
.wrapping_sub(1);
let idx_tree = u64::from_be_bytes(idx_tree_bytes) & mask;
let idx_leaf = u32::from_be_bytes(idx_leaf_bytes) & ((1 << P::HPrime::USIZE) - 1);
(m, idx_tree, idx_leaf)
}

Affects hypertree and FORS test functions (4 call sites).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant