Skip to content

Commit

Permalink
audit: Fixes of issues found during ethernal audit (#111)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mertwole
mertwole authored Aug 22, 2024
1 parent bc48d5b commit 474aa3c
Show file tree
Hide file tree
Showing 18 changed files with 199 additions and 290 deletions.
11 changes: 8 additions & 3 deletions circuits/plonky2_ecdsa/src/gadgets/biguint.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use alloc::vec;
#[cfg(not(test))]
use alloc::vec::Vec;
use core::marker::PhantomData;
use plonky2_u32::gadgets::range_check::range_check_u32_circuit;

use num::{BigUint, Integer, Zero};
use plonky2::field::extension::Extendable;
Expand Down Expand Up @@ -241,7 +242,7 @@ impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilderBiguint<F, D>
let div_num_limbs = if b_len > a_len + 1 {
0
} else {
a_len - b_len + 1
a_len + 1 - b_len
};
let div = self.add_virtual_biguint_target(div_num_limbs);
let rem = self.add_virtual_biguint_target(b_len);
Expand All @@ -254,12 +255,16 @@ impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilderBiguint<F, D>
_phantom: PhantomData,
});

range_check_u32_circuit(self, div.limbs.clone());
range_check_u32_circuit(self, rem.limbs.clone());

let div_b = self.mul_biguint(&div, b);
let div_b_plus_rem = self.add_biguint(&div_b, &rem);
self.connect_biguint(a, &div_b_plus_rem);

let cmp_rem_b = self.cmp_biguint(&rem, b);
self.assert_one(cmp_rem_b.target);
// Check that `b <= rem == false`.
let cmp_rem_b = self.cmp_biguint(b, &rem);
self.assert_zero(cmp_rem_b.target);

(div, rem)
}
Expand Down
2 changes: 1 addition & 1 deletion circuits/plonky2_ed25519/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ serde.workspace = true
anyhow.workspace = true
env_logger.workspace = true
log.workspace = true
rand.workspace = true
rand = { workspace = true, features = ["std", "std_rng"] }
rand_chacha.workspace = true
unroll.workspace = true
keccak-hash.workspace = true
Expand Down
116 changes: 9 additions & 107 deletions circuits/plonky2_ed25519/src/curve/curve_adds.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,9 @@ impl<C: Curve> Add<ProjectivePoint<C>> for ProjectivePoint<C> {
z: z2,
} = rhs;

if z1 == C::BaseField::ZERO {
return rhs;
}
if z2 == C::BaseField::ZERO {
return self;
}
debug_assert!(z1.is_nonzero());
debug_assert!(z2.is_nonzero());

let x1z2 = x1 * z2;
let y1z2 = y1 * z2;
let x2z1 = x2 * z1;
let y2z1 = y2 * z1;

// Check if we're doubling or adding inverses.
if x1z2 == x2z1 {
if y1z2 == y2z1 {
// TODO: inline to avoid redundant muls.
return self.double();
}
if y1z2 == -y2z1 {
return ProjectivePoint::ZERO;
}
}

// From https://www.hyperelliptic.org/EFD/g1p/auto-twisted-projective.html
let a = z1 * z2;
let b = a.square();
let c = x1 * x2;
Expand All @@ -53,7 +32,7 @@ impl<C: Curve> Add<ProjectivePoint<C>> for ProjectivePoint<C> {
let f = b - e;
let g = b + e;
let x3 = a * f * ((x1 + y1) * (x2 + y2) - c - d);
let y3 = a * g * (d + c);
let y3 = a * g * (d - C::A * c);
let z3 = f * g;

ProjectivePoint::nonzero(x3, y3, z3)
Expand All @@ -64,104 +43,27 @@ impl<C: Curve> Add<AffinePoint<C>> for ProjectivePoint<C> {
type Output = ProjectivePoint<C>;

fn add(self, rhs: AffinePoint<C>) -> Self::Output {
let ProjectivePoint {
x: x1,
y: y1,
z: z1,
} = self;
let AffinePoint {
x: x2,
y: y2,
zero: zero2,
} = rhs;

if z1 == C::BaseField::ZERO {
return rhs.to_projective();
}
if zero2 {
return self;
}

let x2z1 = x2 * z1;
let y2z1 = y2 * z1;

// Check if we're doubling or adding inverses.
if x1 == x2z1 {
if y1 == y2z1 {
// TODO: inline to avoid redundant muls.
return self.double();
}
if y1 == -y2z1 {
return ProjectivePoint::ZERO;
}
}

// From https://www.hyperelliptic.org/EFD/g1p/data/shortw/projective/addition/madd-1998-cmo
let u = y2z1 - y1;
let uu = u.square();
let v = x2z1 - x1;
let vv = v.square();
let vvv = v * vv;
let r = vv * x1;
let a = uu * z1 - vvv - r.double();
let x3 = v * a;
let y3 = u * (r - a) - vvv * y1;
let z3 = vvv * z1;
ProjectivePoint::nonzero(x3, y3, z3)
self + rhs.to_projective()
}
}

impl<C: Curve> Add<AffinePoint<C>> for AffinePoint<C> {
type Output = AffinePoint<C>;

fn add(self, rhs: AffinePoint<C>) -> Self::Output {
let AffinePoint {
x: x1,
y: y1,
zero: zero1,
} = self;
let AffinePoint {
x: x2,
y: y2,
zero: zero2,
} = rhs;

if zero1 {
return rhs;
}
if zero2 {
return self;
}

// Check if we're doubling or adding inverses.
if x1 == x2 {
if y1 == y2 {
return self.double();
}
if y1 == -y2 {
return AffinePoint::ZERO;
}
}
let AffinePoint { x: x1, y: y1 } = self;
let AffinePoint { x: x2, y: y2 } = rhs;

let x1x2 = x1 * x2;
let y1y2 = y1 * y2;
let x1y2 = x1 * y2;
let y1x2 = y1 * x2;

let x1y2_add_y1x2 = x1y2 + y1x2;
let y1y2_add_x1x2 = y1y2 + x1x2;

let dx1x2y1y2 = C::D * x1x2 * y1y2;
let one_add_dx1x2y1y2 = C::BaseField::ONE + dx1x2y1y2;
let one_sub_dx1x2y1y2 = C::BaseField::ONE - dx1x2y1y2;

let x3 = x1y2_add_y1x2 / one_add_dx1x2y1y2;
let y3 = y1y2_add_x1x2 / one_sub_dx1x2y1y2;
let x3 = (x1y2 + y1x2) / (C::BaseField::ONE + dx1x2y1y2);
let y3 = (y1y2 - C::A * x1x2) / (C::BaseField::ONE - dx1x2y1y2);

Self {
x: x3,
y: y3,
zero: false,
}
Self { x: x3, y: y3 }
}
}
Loading

0 comments on commit 474aa3c

Please sign in to comment.