Skip to content

Commit

Permalink
P256verify audit fixes (matter-labs#200)
Browse files Browse the repository at this point in the history 
* Validate curve point evaluation output (matter-labs#186)

Not infinity

* Optimize and clarify REDC (matter-labs#187)

* Remove unnecassary variables storing in `currentB` and `currentC` (matter-labs#189)

* Remove redundant check in `projectiveAdd` (matter-labs#195)

* Improve ec constants docs (matter-labs#196)

* Remove unnecessary `r` conversion from Montgomery form (matter-labs#188)

* Fix doc comment mismatch (matter-labs#197)

* Redundant opposite point addition case (matter-labs#198)

* reuse computation (matter-labs#199)

* Add tests for special hash values (matter-labs#202)

---------

Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com>
  • Loading branch information
@ColoCarletti @ilitteri
ColoCarletti and ilitteri authored Nov 8, 2023
1 parent d047659 commit 80f543f
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 41 deletions.
65 changes: 25 additions & 40 deletions precompiles/P256VERIFY.yul
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ object "P256VERIFY" {
// CURVE CONSTANTS

/// @notice Constant function for curve reduced elliptic group order.
/// @dev P is a prime number which defines the field which is a domain of the curve parameters.
/// @dev See https://neuromancer.sk/std/secg/secp256r1 for further details.
/// @return p The curve reduced elliptic group order.
function P() -> p {
p := 0xffffffff00000001000000000000000000000000ffffffffffffffffffffffff
}

/// @notice Constant function for curve subgroup order.
/// @dev N is the order of generator G.
/// @dev See https://neuromancer.sk/std/secg/secp256r1 for further details.
/// @return n The curve subgroup order.
function N() -> n {
Expand Down Expand Up @@ -149,8 +151,7 @@ object "P256VERIFY" {
for {} and(iszero(eq(u, 0x1)), iszero(eq(v, 0x1))) {} {
for {} iszero(and(u, 0x1)) {} {
u := shr(1, u)
let currentB := b
switch and(currentB, 0x1)
switch and(b, 0x1)
case 0 {
b := shr(1, b)
}
Expand All @@ -166,8 +167,7 @@ object "P256VERIFY" {

for {} iszero(and(v, 0x1)) {} {
v := shr(1, v)
let currentC := c
switch and(currentC, 0x1)
switch and(c, 0x1)
case 0 {
c := shr(1, c)
}
Expand Down Expand Up @@ -234,22 +234,14 @@ object "P256VERIFY" {
/// @return S The result of the Montgomery reduction.
function REDC(TLo, THi, n, nPrime) -> S {
let m := mul(TLo, nPrime)
let tHi, tHiOverflowed := overflowingAdd(THi, getHighestHalfOfMultiplication(m, n))
let tHi, tHiOverflowed1 := overflowingAdd(THi, getHighestHalfOfMultiplication(m, n))
let aLo, aLoOverflowed := overflowingAdd(TLo, mul(m, n))
if tHiOverflowed {
// TODO: Check if this addition could overflow.
tHi := add(tHi, sub(0, n))
}
let tHiOverflowed2 := 0
if aLoOverflowed {
tHi, tHiOverflowed := overflowingAdd(tHi, 1)
}
if tHiOverflowed {
// TODO: Check if this addition could overflow.
tHi := add(tHi, sub(0, n))
tHi, tHiOverflowed2 := overflowingAdd(tHi, 1)
}
S := tHi

if iszero(lt(tHi, n)) {
if or(or(tHiOverflowed1, tHiOverflowed2), iszero(lt(tHi, n))) {
S := sub(tHi, n)
}
}
Expand Down Expand Up @@ -439,13 +431,6 @@ object "P256VERIFY" {
function projectiveAdd(xp, yp, zp, xq, yq, zq) -> xr, yr, zr {
let qIsInfinity := projectivePointIsInfinity(xq, yq, zq)
let pIsInfinity := projectivePointIsInfinity(xp, yp, zp)
if and(pIsInfinity, qIsInfinity) {
// Infinity + Infinity = Infinity
xr := 0
yr := MONTGOMERY_ONE_P()
zr := 0
leave
}
if pIsInfinity {
// Infinity + Q = Q
xr := xq
Expand All @@ -460,26 +445,22 @@ object "P256VERIFY" {
zr := zp
leave
}
if eq(montgomeryMul(xp, zq, P(), P_PRIME()), montgomeryMul(xq, zp, P(), P_PRIME())) {
if eq(montgomeryMul(yp, zq, P(), P_PRIME()), montgomeryMul(yq, zp, P(), P_PRIME())) {
// P + P = 2P
xr, yr, zr := projectiveDouble(xp, yp, zp)
leave
}
// P + (-P) = Infinity
xr := 0
yr := MONTGOMERY_ONE_P()
zr := 0
leave
}

// P1 + P2 = P3
let t0 := montgomeryMul(yp, zq, P(), P_PRIME())
let t1 := montgomeryMul(yq, zp, P(), P_PRIME())
let t := montgomerySub(t0, t1, P())
let u0 := montgomeryMul(xp, zq, P(), P_PRIME())
let u1 := montgomeryMul(xq, zp, P(), P_PRIME())
let u := montgomerySub(u0, u1, P())

// t = (yp*zq - yq*zp); u = (xp*zq - xq*zp)
if iszero(or(t, u)) {
// P + P = 2P
xr, yr, zr := projectiveDouble(xp, yp, zp)
leave
}

// P1 + P2 = P3
let u2 := montgomeryMul(u, u, P(), P_PRIME())
let u3 := montgomeryMul(u2, u, P(), P_PRIME())
let v := montgomeryMul(zp, zq, P(), P_PRIME())
Expand All @@ -494,8 +475,8 @@ object "P256VERIFY" {
/// @param xq The x coordinate of the point Q in projective coordinates in Montgomery form.
/// @param yq The y coordinate of the point Q in projective coordinates in Montgomery form.
/// @param zq The z coordinate of the point Q in projective coordinates in Montgomery form.
/// @param t0 The scalar to multiply the point Q by.
/// @param t1 The scalar to multiply the generator G by.
/// @param t0 The scalar to multiply the generator G by.
/// @param t1 The scalar to multiply the point Q by.
/// @return xr The x coordinate of the resulting point R in projective coordinates in Montgomery form.
/// @return yr The y coordinate of the resulting point R in projective coordinates in Montgomery form.
/// @return zr The z coordinate of the resulting point R in projective coordinates in Montgomery form.
Expand Down Expand Up @@ -595,6 +576,7 @@ object "P256VERIFY" {
// TODO: Check if r, s, s1, t0 and t1 operations are optimal in Montgomery form or not

hash := intoMontgomeryForm(hash, N(), N_PRIME(), R2_MOD_N())
let rTmp := r
r := intoMontgomeryForm(r, N(), N_PRIME(), R2_MOD_N())
s := intoMontgomeryForm(s, N(), N_PRIME(), R2_MOD_N())

Expand All @@ -604,16 +586,19 @@ object "P256VERIFY" {
let t1 := outOfMontgomeryForm(montgomeryMul(r, s1, N(), N_PRIME()), N(), N_PRIME())

let xr, yr, zr := shamirLinearCombination(x, y, z, t0, t1)
if iszero(zr) {
mstore(0, 0)
return(0, 32)
}

// As we only need xr in affine form, we can skip transforming the `y` coordinate.
let z_inv := montgomeryModularInverse(zr, P(), R2_MOD_P())
xr := montgomeryMul(xr, z_inv, P(), P_PRIME())
xr := outOfMontgomeryForm(xr, P(), P_PRIME())

r := outOfMontgomeryForm(r, N(), N_PRIME())
xr := mod(xr, N())

mstore(0, eq(xr, r))
mstore(0, eq(xr, rTmp))
return(0, 32)
}
}
Expand Down
100 changes: 99 additions & 1 deletion tests/tests/p256verify_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ async fn p256verify_valid_signature_two() {
}

#[tokio::test]
async fn p256verify_invalid_signature() {
async fn p256verify_invalid_signature_one() {
let era_response = era_call(
P256VERIFTY_PRECOMPILE_ADDRESS,
None,
Expand Down Expand Up @@ -147,3 +147,101 @@ async fn p256verify_public_key_not_in_curve() {

assert_eq!(era_response, EXECUTION_REVERTED)
}

#[tokio::test]
async fn p256verify_invalid_signature_two() {
let era_response = era_call(
P256VERIFTY_PRECOMPILE_ADDRESS,
None,
Some(Bytes::from(hex::decode("5ad83880e16658d7521d4e878521defaf6b43dec1dbd69e514c09ab8f1f2ffe25ad83880e16658d7521d4e878521defaf6b43dec1dbd69e514c09ab8f1f2ffe2871c518be8c56e7f5c901933fdab317efafc588b3e04d19d9a27b29aad8d9e696b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c296b01cbd1c01e58065711814b583f061e9d431cca994cea1313449bf97c840ae0a").unwrap())),
)
.await
.unwrap();
let (era_output, gas_used) = parse_call_result(&era_response);
write_p256verify_gas_result(gas_used);
assert_eq!(era_output, Bytes::from(RESPONSE_INVALID))
}

#[tokio::test]
async fn p256verify_hash_edge_case_valid_1() {
let era_response = era_call(
P256VERIFTY_PRECOMPILE_ADDRESS,
None,
Some(Bytes::from(hex::decode("00000000000000000000000000000000000000000000000000000000000000018c47ad0afe2e980cc144632bdc1d442c34fd234661f9cb983e66a59abc1eed05844c7bf016cf7cb4ae740fac63cc8ca08e6db74890d94db8954c52fd77bf040c6b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2964fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f5").unwrap())),
)
.await
.unwrap();
let (era_output, gas_used) = parse_call_result(&era_response);
write_p256verify_gas_result(gas_used);
assert_eq!(era_output, Bytes::from(RESPONSE_VALID))
}

#[tokio::test]
async fn p256verify_hash_edge_case_valid_2() {
let era_response = era_call(
P256VERIFTY_PRECOMPILE_ADDRESS,
None,
Some(Bytes::from(hex::decode("00000000000000000000000000000000000000000000000000000000000000016b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2966b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2976b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2964fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f5").unwrap())),
)
.await
.unwrap();
let (era_output, gas_used) = parse_call_result(&era_response);
write_p256verify_gas_result(gas_used);
assert_eq!(era_output, Bytes::from(RESPONSE_VALID))
}

#[tokio::test]
async fn p256verify_hash_edge_case_valid_3() {
let era_response = era_call(
P256VERIFTY_PRECOMPILE_ADDRESS,
None,
Some(Bytes::from(hex::decode("00000000000000000000000000000000000000000000000000000000000000006b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2966b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2966b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2964fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f5").unwrap())),
)
.await
.unwrap();
let (era_output, gas_used) = parse_call_result(&era_response);
write_p256verify_gas_result(gas_used);
assert_eq!(era_output, Bytes::from(RESPONSE_VALID))
}

#[tokio::test]
async fn p256verify_hash_edge_case_valid_4() {
let era_response = era_call(
P256VERIFTY_PRECOMPILE_ADDRESS,
None,
Some(Bytes::from(hex::decode("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff6b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2966b17d1f3e12c4246f8bce6e563a440f2ba1c82d386d3951c00e76e82dc359d446b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2964fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f5").unwrap())),
)
.await
.unwrap();
let (era_output, gas_used) = parse_call_result(&era_response);
write_p256verify_gas_result(gas_used);
assert_eq!(era_output, Bytes::from(RESPONSE_VALID))
}

#[tokio::test]
async fn p256verify_hash_edge_case_valid_5() {
let era_response = era_call(
P256VERIFTY_PRECOMPILE_ADDRESS,
None,
Some(Bytes::from(hex::decode("ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc6325516b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2966b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2966b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2964fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f5").unwrap())),
)
.await
.unwrap();
let (era_output, gas_used) = parse_call_result(&era_response);
write_p256verify_gas_result(gas_used);
assert_eq!(era_output, Bytes::from(RESPONSE_VALID))
}

#[tokio::test]
async fn p256verify_hash_edge_case_valid_6() {
let era_response = era_call(
P256VERIFTY_PRECOMPILE_ADDRESS,
None,
Some(Bytes::from(hex::decode("ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc6325506b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2966b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2956b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2964fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f5").unwrap())),
)
.await
.unwrap();
let (era_output, gas_used) = parse_call_result(&era_response);
write_p256verify_gas_result(gas_used);
assert_eq!(era_output, Bytes::from(RESPONSE_VALID))
}

0 comments on commit 80f543f

Please sign in to comment.