Skip to content

Commit

Permalink
Avoid index slicing
Browse files Browse the repository at this point in the history
  • Loading branch information
dvdplm committed Dec 9, 2024
1 parent 806b393 commit 20938a7
Show file tree
Hide file tree
Showing 17 changed files with 116 additions and 54 deletions.
1 change: 1 addition & 0 deletions synedrion/src/cggmp21/aux_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,7 @@ impl<P: SchemeParams, I: PartyId + Serialize> Round<I> for Round3<P, I> {
}
}

#[allow(clippy::indexing_slicing)]
#[cfg(test)]
mod tests {
use alloc::collections::BTreeSet;
Expand Down
19 changes: 15 additions & 4 deletions synedrion/src/cggmp21/interactive_signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -887,18 +887,28 @@ impl<P: SchemeParams, I: PartyId> Round<I> for Round3<P, I> {
.round2_artifacts
.into_iter()
.map(|(id, artifact)| {
let cap_k = self
.all_cap_k
.get(&id)
.ok_or_else(|| LocalError::new("id={id:?} is missing in all_cap_k"))?
.clone();
let hat_cap_d_received = self
.hat_cap_ds
.get(&id)
.ok_or_else(|| LocalError::new("id={id:?} is missing in hat_cap_ds"))?
.clone();
let values = PresigningValues {
hat_beta: artifact.hat_beta,
hat_r: artifact.hat_r,
hat_s: artifact.hat_s,
cap_k: self.all_cap_k[&id].clone(),
hat_cap_d_received: self.hat_cap_ds[&id].clone(),
cap_k,
hat_cap_d_received,
hat_cap_d: artifact.hat_cap_d,
hat_cap_f: artifact.hat_cap_f,
};
(id, values)
Ok((id, values))
})
.collect();
.collect::<Result<_, LocalError>>()?;

let presigning_data = PresigningData {
nonce,
Expand Down Expand Up @@ -1350,6 +1360,7 @@ impl<P: SchemeParams, I: PartyId> Round<I> for SigningErrorRound<P, I> {
}
}

#[allow(clippy::indexing_slicing)]
#[cfg(test)]
mod tests {
use alloc::collections::BTreeSet;
Expand Down
1 change: 1 addition & 0 deletions synedrion/src/cggmp21/key_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@ impl<P: SchemeParams, I: PartyId> Round<I> for Round3<P, I> {
}
}

#[allow(clippy::indexing_slicing)]
#[cfg(test)]
mod tests {
use alloc::collections::{BTreeMap, BTreeSet};
Expand Down
1 change: 1 addition & 0 deletions synedrion/src/cggmp21/key_refresh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,7 @@ impl<P: SchemeParams, I: PartyId> Round<I> for Round3<P, I> {
}
}

#[allow(clippy::indexing_slicing)]
#[cfg(test)]
mod tests {

Expand Down
31 changes: 24 additions & 7 deletions synedrion/src/cggmp21/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ use crate::{
#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct PaillierTest;

#[allow(clippy::indexing_slicing)]
const fn upcast_uint<const N1: usize, const N2: usize>(value: K256Uint<N1>) -> K256Uint<N2> {
assert!(N2 >= N1, "Upcast target must be bigger than the upcast candidate");
let mut result_words = [0; N2];
let mut i = 0;
let words = value.as_words();
while i < N1 {
result_words[i] = value.as_words()[i];
result_words[i] = words[i];
i += 1;
}
K256Uint::from_words(result_words)
Expand Down Expand Up @@ -132,8 +134,14 @@ pub trait SchemeParams: Debug + Clone + Send + PartialEq + Eq + Send + Sync + 's
let uint_len = repr.as_ref().len();
let scalar_len = scalar_bytes.len();

debug_assert!(uint_len >= scalar_len);
repr.as_mut()[uint_len - scalar_len..].copy_from_slice(&scalar_bytes);
debug_assert!(
uint_len >= scalar_len,
"PaillierParams::Uint is expected to be bigger than a Scalar"

Check warning on line 139 in synedrion/src/cggmp21/params.rs

View check run for this annotation

Codecov / codecov/patch

synedrion/src/cggmp21/params.rs#L139

Added line #L139 was not covered by tests
);
repr.as_mut()
.get_mut(uint_len - scalar_len..)
.expect("PaillierParams::Uint is expected to be bigger than a Scalar")
.copy_from_slice(&scalar_bytes);
<Self::Paillier as PaillierParams>::Uint::from_be_bytes(repr)
}

Expand Down Expand Up @@ -162,8 +170,12 @@ pub trait SchemeParams: Debug + Clone + Send + PartialEq + Eq + Send + Sync + 's
let scalar_len = Scalar::repr_len();

// Can unwrap here since the value is within the Scalar range
Scalar::try_from_bytes(&repr.as_ref()[uint_len - scalar_len..])
.expect("the value was reduced modulo `CURVE_ORDER`, so it's a valid curve scalar")
Scalar::try_from_bytes(
repr.as_ref()
.get(uint_len - scalar_len..)
.expect("Uint is assumed to be bigger than Scalar"),
)
.expect("the value was reduced modulo `CURVE_ORDER`, so it's a valid curve scalar")
}

/// Converts a `Signed`-wrapped integer to the associated curve scalar type.
Expand All @@ -181,8 +193,13 @@ pub trait SchemeParams: Debug + Clone + Send + PartialEq + Eq + Send + Sync + 's
let scalar_len = Scalar::repr_len();

// Can unwrap here since the value is within the Scalar range
Scalar::try_from_bytes(&repr.as_ref()[uint_len - scalar_len..])
.expect("the value was reduced modulo `CURVE_ORDER`, so it's a valid curve scalar")
Scalar::try_from_bytes(
repr.as_ref()
.get(uint_len - scalar_len..)
// TODO(dp): Do I need a better proof that this is true?
.expect("WideUint is assumed to be bigger than Scalar"),
)
.expect("the value was reduced modulo `CURVE_ORDER`, so it's a valid curve scalar")
}

/// Converts a `Signed`-wrapped wide integer to the associated curve scalar type.
Expand Down
11 changes: 6 additions & 5 deletions synedrion/src/cggmp21/sigma/mod_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,15 @@ impl<P: SchemeParams> ModProof<P> {

let (omega_mod_p, omega_mod_q) = sk.rns_split(&commitment.0);

let proof = (0..challenge.0.len())
.map(|i| {
let proof = challenge
.0
.iter()
.map(|y| {
let mut y_sqrt = None;
let mut found_a = false;
let mut found_b = false;
for (a, b) in [(false, false), (false, true), (true, false), (true, true)].iter() {
let y = challenge.0[i];
let (mut y_mod_p, mut y_mod_q) = sk.rns_split(&y);
let (mut y_mod_p, mut y_mod_q) = sk.rns_split(y);
if *a {
y_mod_p = -y_mod_p;
y_mod_q = -y_mod_q;
Expand All @@ -114,7 +115,7 @@ impl<P: SchemeParams> ModProof<P> {

let y_4th = sk.rns_join(&y_4th_parts);

let y = challenge.0[i].to_montgomery(pk.monty_params_mod_n());
let y = y.to_montgomery(pk.monty_params_mod_n());
let sk_inv_modulus = sk.inv_modulus();
let z = y.pow_bounded(sk_inv_modulus.expose_secret());

Expand Down
10 changes: 4 additions & 6 deletions synedrion/src/cggmp21/sigma/prm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,10 @@ impl<P: SchemeParams> PrmProof<P> {
return false;
}

for i in 0..challenge.0.len() {
let z = self.proof[i];
let e = challenge.0[i];
let a = self.commitment.0[i].to_montgomery(monty_params);
let pwr = setup.base().pow_bounded(&z);
let test = if e { pwr == a * setup.power() } else { pwr == a };
for ((e, z), a) in challenge.0.iter().zip(self.proof.iter()).zip(self.commitment.0.iter()) {
let a = a.to_montgomery(monty_params);
let pwr = setup.base().pow_bounded(z);
let test = if *e { pwr == a * setup.power() } else { pwr == a };
if !test {
return false;
}
Expand Down
1 change: 1 addition & 0 deletions synedrion/src/cggmp21/signing_malicious.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ impl<P: SchemeParams, Id: PartyId> Misbehaving<Id, Behavior> for MaliciousSignin

type MaliciousSigning<P, Id> = MisbehavingEntryPoint<Id, Behavior, MaliciousSigningOverride<P>>;

#[allow(clippy::indexing_slicing)]
#[test]
fn execute_signing() {
let signers = (0..3).map(TestSigner::new).collect::<Vec<_>>();
Expand Down
1 change: 1 addition & 0 deletions synedrion/src/curve/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ impl Scalar {
Self(*sk.as_nonzero_scalar().as_ref())
}

// TODO(dp): Does this assume big endian order?
pub(crate) fn try_from_bytes(bytes: &[u8]) -> Result<Self, String> {
let arr = GenericArray::<u8, FieldBytesSize<Secp256k1>>::from_exact_iter(bytes.iter().cloned())
.ok_or("Invalid length of a curve scalar")?;
Expand Down
12 changes: 9 additions & 3 deletions synedrion/src/paillier/rsa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
fn random_paillier_blum_prime<P: PaillierParams>(rng: &mut impl CryptoRngCore) -> P::HalfUint {
loop {
let prime = P::HalfUint::generate_prime_with_rng(rng, P::PRIME_BITS);
if prime.as_ref()[0].0 & 3 == 3 {
if prime.as_ref().first().expect("First Limb exists").0 & 3 == 3 {
return prime;
}
}
Expand All @@ -34,8 +34,14 @@ pub(crate) struct SecretPrimesWire<P: PaillierParams> {
impl<P: PaillierParams> SecretPrimesWire<P> {
/// A single constructor to check the invariants
fn new(p: Secret<P::HalfUint>, q: Secret<P::HalfUint>) -> Self {
debug_assert!(p.expose_secret().as_ref()[0].0 & 3 == 3, "p must be 3 mod 4");
debug_assert!(q.expose_secret().as_ref()[0].0 & 3 == 3, "q must be 3 mod 4");
debug_assert!(
p.expose_secret().as_ref().first().expect("First Limb exists").0 & 3 == 3,
"p must be 3 mod 4"

Check warning on line 39 in synedrion/src/paillier/rsa.rs

View check run for this annotation

Codecov / codecov/patch

synedrion/src/paillier/rsa.rs#L39

Added line #L39 was not covered by tests
);
debug_assert!(
q.expose_secret().as_ref().first().expect("First Limb exists").0 & 3 == 3,
"q must be 3 mod 4"

Check warning on line 43 in synedrion/src/paillier/rsa.rs

View check run for this annotation

Codecov / codecov/patch

synedrion/src/paillier/rsa.rs#L43

Added line #L43 was not covered by tests
);
Self { p, q }
}

Expand Down
4 changes: 2 additions & 2 deletions synedrion/src/tools/bitvec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ impl BitVec {
impl BitXorAssign<&BitVec> for BitVec {
fn bitxor_assign(&mut self, rhs: &BitVec) {
assert!(self.0.len() == rhs.0.len());
for i in 0..self.0.len() {
self.0[i] ^= rhs.0[i];
for (lhs, rhs) in self.0.iter_mut().zip(rhs.0.iter()) {
*lhs ^= rhs
}
}
}
27 changes: 17 additions & 10 deletions synedrion/src/tools/hashing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,25 +155,32 @@ where
let backend_modulus = modulus.as_ref();

let n_bits = backend_modulus.bits_vartime();
let n_bytes = (n_bits + 7) / 8; // ceiling division by 8
let n_bytes = n_bits.div_ceil(8) as usize;

// If the number of bits is not a multiple of 8,
// use a mask to zeroize the high bits in the gererated random bytestring,
// so that we don't have to reject too much.
// If the number of bits is not a multiple of 8, use a mask to zeroize the high bits in the
// gererated random bytestring, so that we don't have to reject too much.
let mask = if n_bits & 7 != 0 {
(1 << (n_bits & 7)) - 1
} else {
u8::MAX
};

// TODO(dp): this uses `to_le_bytes` (little-endian) but then the original code masks out bits
// from the *last* byte, so it'd mask out the *low* bits yeah? Changing to mask the first byte
// but this could use a test.
let mut bytes = T::zero().to_le_bytes();
loop {
reader.read(&mut (bytes.as_mut()[0..n_bytes as usize]));
bytes.as_mut()[n_bytes as usize - 1] &= mask;
let n = T::from_le_bytes(bytes);

if n.ct_lt(backend_modulus).into() {
return n;
if let Some(buf) = bytes.as_mut().get_mut(0..n_bytes) {
reader.read(buf);
bytes.as_mut().first_mut().map(|byte| {
*byte &= mask;
Some(byte)
});
let n = T::from_le_bytes(bytes);

if n.ct_lt(backend_modulus).into() {
return n;
}
}
}
}
20 changes: 13 additions & 7 deletions synedrion/src/tools/sss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use alloc::{
vec::Vec,
};
use core::ops::{Add, Mul};
use manul::session::LocalError;

use rand_core::CryptoRngCore;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -34,13 +35,15 @@ fn evaluate_polynomial<T>(coeffs: &[T], x: &Scalar) -> T
where
T: Copy + Add<T, Output = T> + for<'a> Mul<&'a Scalar, Output = T>,
{
assert!(coeffs.len() > 1, "Expected coefficients to be non-empty");
// Evaluate in reverse to save on multiplications.
// Basically: a0 + a1 x + a2 x^2 + a3 x^3 == (((a3 x) + a2) x + a1) x + a0
let mut res = coeffs[coeffs.len() - 1];
for i in (0..(coeffs.len() - 1)).rev() {
res = res * x + coeffs[i];
}
res

let (acc, coeffs) = coeffs.split_last().expect("Coefficients is not empty");
coeffs.iter().rev().fold(*acc, |mut acc, coeff| {
acc = acc * x + *coeff;
acc
})
}

#[derive(Debug, ZeroizeOnDrop)]
Expand Down Expand Up @@ -73,8 +76,10 @@ impl PublicPolynomial {
evaluate_polynomial(&self.0, &x.0)
}

pub fn coeff0(&self) -> Point {
self.0[0]
pub fn coeff0(&self) -> Result<&Point, LocalError> {
self.0
.first()
.ok_or_else(|| LocalError::new("Invalid PublicPolynomial"))
}
}

Expand Down Expand Up @@ -116,6 +121,7 @@ pub(crate) fn shamir_join_points(pairs: &BTreeMap<ShareId, Point>) -> Point {
.sum()
}

#[allow(clippy::indexing_slicing)]
#[cfg(test)]
mod tests {
use rand_core::OsRng;
Expand Down
10 changes: 8 additions & 2 deletions synedrion/src/uint/bounded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ where
fn from(val: Bounded<T>) -> Self {
let repr = val.as_ref().to_be_bytes();
let bound_bytes = (val.bound() + 7) / 8;
let slice = &repr.as_ref()[(repr.as_ref().len() - bound_bytes as usize)..];
let slice = repr
.as_ref()
.get((repr.as_ref().len() - bound_bytes as usize)..)
.expect("val has a valid bound that was checked when it was created");
Self {
bound: val.bound(),
bytes: slice.into(),
Expand All @@ -52,7 +55,10 @@ where
));
}

repr.as_mut()[(repr_len - bytes_len)..].copy_from_slice(&val.bytes);
repr.as_mut()
.get_mut((repr_len - bytes_len)..)
.expect("Just checked that val's data all fit in a T")
.copy_from_slice(&val.bytes);
let abs_value = T::from_be_bytes(repr);

Self::new(abs_value, val.bound).ok_or_else(|| "Invalid values for the signed integer".into())
Expand Down
12 changes: 6 additions & 6 deletions synedrion/src/uint/traits.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crypto_bigint::{
modular::MontyForm,
nlimbs,
subtle::{ConditionallySelectable, CtOption},
Encoding, Integer, Invert, PowBoundedExp, RandomMod, Square, Zero, U1024, U2048, U4096, U512, U8192,
Encoding, Integer, Invert, Limb, PowBoundedExp, RandomMod, Square, Zero, U1024, U2048, U4096, U512, U8192,
};

use crate::uint::{Bounded, Signed};
Expand Down Expand Up @@ -220,10 +219,11 @@ impl HasWide for U4096 {
}
}

pub type U512Mod = MontyForm<{ nlimbs!(512) }>;
pub type U1024Mod = MontyForm<{ nlimbs!(1024) }>;
pub type U2048Mod = MontyForm<{ nlimbs!(2048) }>;
pub type U4096Mod = MontyForm<{ nlimbs!(4096) }>;
// TODO(dp): Suggest crypto-bigint update nlimbs! macro.
pub type U512Mod = MontyForm<{ 512u32.div_ceil(Limb::BITS) as usize }>;
pub type U1024Mod = MontyForm<{ 1024u32.div_ceil(Limb::BITS) as usize }>;
pub type U2048Mod = MontyForm<{ 2048u32.div_ceil(Limb::BITS) as usize }>;
pub type U4096Mod = MontyForm<{ 4096u32.div_ceil(Limb::BITS) as usize }>;

impl ToMontgomery for U512 {}
impl ToMontgomery for U1024 {}
Expand Down
1 change: 1 addition & 0 deletions synedrion/src/www02/entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ fn apply_tweaks_private(private_key: SigningKey, tweaks: &[PrivateKeyBytes]) ->
Ok(private_key)
}

#[allow(clippy::indexing_slicing)]
#[cfg(test)]
mod tests {
use alloc::collections::BTreeSet;
Expand Down
Loading

0 comments on commit 20938a7

Please sign in to comment.