Skip to content

Commit

Permalink
Merge pull request #415 from 0xPolygonHermez/fix/p256-audit
Browse files Browse the repository at this point in the history
Modifications p256verify
  • Loading branch information
laisolizq authored Nov 7, 2024
2 parents b481ddc + f3b321b commit 1a69e22
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 93 deletions.
19 changes: 8 additions & 11 deletions main/p256verify/dblScalarMulSecp256r1.zkasm
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
;; dblScalarMulSecp256r1:
;; in: Points p1 = (p1_x, p1_y), p2 = (p2_x, p2_y) and scalars k1, k2
;; out:
;; · p3 = (p3_x, p3_y) ∈ E(Fp)
;; · p3 = [k1]·P1 + [k2]·P2 = (p3_x, p3_y) ∈ E(Fp)
;; · Variable p3_is_zero = 0 or 1, to indicate whether p3 is the point at infinity or not
;;
;; PRE: p1 = (p1_x, p1_y), p2 = (p2_x, p2_y) are in E(Fp)\{𝒪} and p1_x,p1_y,p2_x,p2_y are in Fp.
; Moreover, k1,k2 are in Fn and they cannot be both zero.
;; Moreover, k1,k2 are in Fn and they cannot be both zero.
;; POST: The resulting coordinates p3_x,p3_y are in Fp.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

Expand Down Expand Up @@ -59,9 +59,8 @@ dblScalarMulSecp256r1:

; receive the maximum length (minus one) between the binary representations of k1 and k2
; Note that 0 <= max(len(k1)-1,len(k2)-1) <= 255
$0{receiveLen(mem.dblScalarMulSecp256r1_k1, mem.dblScalarMulSecp256r1_k2)} => A,RR
256 => B
1 :LT
$0{receiveLen(mem.dblScalarMulSecp256r1_k1, mem.dblScalarMulSecp256r1_k2)} => RR :JMPN(failAssert)
255 - RR :JMPN(failAssert)

; start the acummulator of k1 and k2
0 => RCX :MSTORE(dblScalarMulSecp256r1_acum_k1)
Expand All @@ -87,11 +86,11 @@ dblScalarMulSecp256r1:
${B == D} :JMPNZ(dblScalarMulSecp256r1SameInitialPoints)
; [steps: 15, bin: 1, arith: 0]

; verify path p1_y != p2_y <==> p1_y = -p2_y <==> p1_y + p2_y = 0
; verify path p1_y != p2_y <==> p1_y = -p2_y (mod p) <==> p1_y + p2_y = 0 (mod p) <==> p1_y + p2_y ∊ {0,SECP256R1_P}
; Note: In this path we could use a binary, but we use an arith
; because in this path we do not perform point arithmetic

; y must be distinct from 0, because there are no points with y = 0
; y must be distinct from 0, because there are no points with y = 0, then p1_y + p2_y = SECP256R1_P

; check p1_y + p2_y = SECP256R1_P
B => A ; A = p1_y
Expand Down Expand Up @@ -296,13 +295,11 @@ dblScalarMulSecp256r1_x_equals_before_add:
1 :MSTORE(dblScalarMulSecp256r1_p3_zero), JMP(dblScalarMulSecp256r1_double)

dblScalarMulSecp256r1_same_point_to_add:
; must check really are equals, use MLOAD as ASSERT
; ASSERT(D == dblScalarMulSecp256r1_p3_y)

; verify path p3_y == p_y (D)
D :MLOAD(dblScalarMulSecp256r1_p3_y)

C => A
D => B

; (A,B) * 2 = (E, op)
${xDblPointEc_secp256r1(A,B)} => E :MSTORE(dblScalarMulSecp256r1_p3_x)
${yDblPointEc_secp256r1(A,B)} :ARITH_SECP256R1_ECADD_SAME, MSTORE(dblScalarMulSecp256r1_p3_y), JMP(dblScalarMulSecp256r1_double)
Expand Down
77 changes: 43 additions & 34 deletions main/p256verify/p256verify.zkasm
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,13 @@
;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

; Note: If hash = 0, then one can forge a valid signature:
; Note1 ZeroHash: If hash = 0, then one can forge a valid signature:
; https://crypto.stackexchange.com/questions/50279/how-should-ecdsa-handle-the-null-hash
; We anyways do not check for this case, as it happens with negligible probability
; We anyways do not check for this case, as it happens with negligible probability.

; Note2 Malleability: Given a signature (r,s) over a message m, the signature (r,N-s) is also valid for the same message:
; https://github.com/cosmos/cosmos-sdk/issues/9723, https://eips.ethereum.org/EIPS/eip-2
; This opens the door to replay attacks. This is not checked in this precompiled.

; inputs
VAR GLOBAL p256verify_hash
Expand All @@ -26,6 +30,7 @@ VAR GLOBAL p256verify_x
VAR GLOBAL p256verify_y

VAR GLOBAL p256verify_x3
VAR GLOBAL p256verify_y2
VAR GLOBAL p256verify_s_inv
VAR GLOBAL p256verify_u1
VAR GLOBAL p256verify_u2
Expand All @@ -35,16 +40,17 @@ VAR GLOBAL p256verify_RR
INCLUDE "constants.zkasm"

; ERROR CODES (B)
; 0 - no error
; 1 - r is zero
; 2 - r is too big
; 3 - s is zero
; 4 - s is too big
; 5 - x is too big
; 6 - y is too big
; 7 - PK is the point at infinity
; 8 - PK is not in the curve
; 9 - result is the point at infinity
; 0 - no error
; 1 - r is zero
; 2 - r is too big
; 3 - s is zero
; 4 - s is too big
; 5 - x is too big
; 6 - y is too big
; 7 - PK is the point at infinity
; 8 - PK is not in the curve
; 9 - result is the point at infinity
; 10 - signature not verified

; RESOURCES:
; · r is zero: [steps: 13, bin: 1, arith: 0]
Expand Down Expand Up @@ -76,31 +82,31 @@ p256verify_correctness_checks:
; 1] Check that r in [1,N-1]
0n => A
$ => B :MLOAD(p256verify_r)
$ :EQ, JMPC(p256verify_r_is_zero)
$ :EQ, JMPC(p256verify_error_r_is_zero)
%SECP256R1_N_MINUS_ONE => A
$ :LT, JMPC(p256verify_r_is_too_big)
$ :LT, JMPC(p256verify_error_r_is_too_big)

; 2] Check that s in [1,N-1]
$ => B :MLOAD(p256verify_s)
$ :LT, JMPC(p256verify_s_is_too_big)
$ :LT, JMPC(p256verify_error_s_is_too_big)
0n => A
$ :EQ, JMPC(p256verify_s_is_zero)
$ :EQ, JMPC(p256verify_error_s_is_zero)

; 3] Check that x in [0,P-1]
%SECP256R1_P_MINUS_ONE => A
$ => B :MLOAD(p256verify_x)
$ :LT, JMPC(p256verify_x_is_too_big)
$ :LT, JMPC(p256verify_error_x_is_too_big)

; 4] Check that y in [0,P-1]
$ => B :MLOAD(p256verify_y)
$ :LT, JMPC(p256verify_y_is_too_big)
$ :LT, JMPC(p256verify_error_y_is_too_big)

; 5] Check whether PK = (x,y) is the point at infinity 𝒪
0n => B
$ => A :MLOAD(p256verify_x)
$ :EQ, JMPNC(p256verify_pk_not_point_at_infinity)
$ => A :MLOAD(p256verify_y)
$ :EQ, JMPC(p256verify_pk_point_at_infinity)
$ :EQ, JMPC(p256verify_error_pk_point_at_infinity)
; [steps: 29, bin: 8, arith: 0]

p256verify_pk_not_point_at_infinity:
Expand All @@ -123,14 +129,14 @@ p256verify_pk_not_point_at_infinity:

$ => A :MLOAD(p256verify_x3), CALL(addFpSecp256r1) ; [in: A,C, out: C] [steps: 10, bin: 1, arith: 2]
; C = x³ + a·x + b
C :MSTORE(p256verify_x3)
C :MSTORE(p256verify_y2)

$ => A :MLOAD(p256verify_y), CALL(squareFpSecp256r1) ; [in: A, out: A] [steps: 11, bin: 1, arith: 2]
; A = y²

; 1.2] Check if LHS == RHS
$ => B :MLOAD(p256verify_x3)
$ :EQ, JMPNC(p256verify_pk_not_in_curve)
$ => B :MLOAD(p256verify_y2)
$ :EQ, JMPNC(p256verify_error_pk_not_in_curve)
; [steps: 100, bin: 15, arith: 12]

p256verify_scalars:
Expand Down Expand Up @@ -163,12 +169,12 @@ p256verify_ecp:
$ => A :MLOAD(p256verify_x)
A :MSTORE(dblScalarMulSecp256r1_p2_x)
$ => A :MLOAD(p256verify_y)
A :MSTORE(dblScalarMulSecp256r1_p2_y), CALL(dblScalarMulSecp256r1) ; [steps: 6459, bin: 1, arith: 513]
A :MSTORE(dblScalarMulSecp256r1_p2_y), CALL(dblScalarMulSecp256r1) ; [in: [k1, p1_x, p1_y, k2, p2_x, p2_y], out: [p3_is_zero, p3_x, p3_y]] [steps: 6459, bin: 1, arith: 513]
; [steps: 7707, bin: 19, arith: 531]

p256verify_verification:
; check if result of dblScalarMulSecp256k1 is point at infinity
$ :MLOAD(dblScalarMulSecp256r1_p3_zero), JMPNZ(p256verify_result_point_at_infinity)
$ :MLOAD(dblScalarMulSecp256r1_p3_zero), JMPNZ(p256verify_error_result_point_at_infinity)

; check if the x-coordinate of the result is equal to r
; perform modular reduction is p3_x >= SECP256R1_N
Expand All @@ -187,41 +193,44 @@ p256verify_assign_a:

p256verify_final_check:
$ => B :MLOAD(p256verify_r)
$ => A :EQ
$ => A :EQ, JMPNC(p256verify_error_signature_not_verified)
; A = 1 if the signature is valid; 0 otherwise

; program ended with no errors
0 => B :JMP(p256verify_end)
; till the end -> [steps: 7718, bin: 22, arith: 531]

; ERRORS
p256verify_r_is_zero:
p256verify_error_r_is_zero:
1 => B :JMP(p256verify_error)

p256verify_r_is_too_big:
p256verify_error_r_is_too_big:
2 => B :JMP(p256verify_error)

p256verify_s_is_zero:
p256verify_error_s_is_zero:
3 => B :JMP(p256verify_error)

p256verify_s_is_too_big:
p256verify_error_s_is_too_big:
4 => B :JMP(p256verify_error)

p256verify_x_is_too_big:
p256verify_error_x_is_too_big:
5 => B :JMP(p256verify_error)

p256verify_y_is_too_big:
p256verify_error_y_is_too_big:
6 => B :JMP(p256verify_error)

p256verify_pk_point_at_infinity:
p256verify_error_pk_point_at_infinity:
7 => B :JMP(p256verify_error)

p256verify_pk_not_in_curve:
p256verify_error_pk_not_in_curve:
8 => B :JMP(p256verify_error)

p256verify_result_point_at_infinity:
p256verify_error_result_point_at_infinity:
9 => B :JMP(p256verify_error)

p256verify_error_signature_not_verified:
10 => B :JMP(p256verify_end)

p256verify_error:
0 => A

Expand Down
8 changes: 5 additions & 3 deletions main/precompiled/pre-p256verify.zkasm
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@ funcP256VERIFY:
$ => D :MLOAD(readXFromCalldataResult)
; read y [32 bytes]
E + 32 => E :MSTORE(readXFromCalldataOffset), CALL(readFromCalldataOffset); in: [readXFromCalldataOffset: offset value, readXFromCalldataLength: length value], out: [readXFromCalldataResult: result value]
$ => E :MLOAD(readXFromCalldataResult), CALL(p256verify) ;in: [A: hash, B: r, C: s, D: x, E: y], out: [A: result, B: result_code]
$ => E :MLOAD(readXFromCalldataResult)

; call p256verify
:CALL(p256verify) ;in: [A: hash, B: r, C: s, D: x, E: y], out: [A: result, B: result_code]
B :JMPNZ(endP256VERIFYFail)
A :JMPZ(preEndP256VERIFY)

; write result p256verify into memory
0 => E
A :MSTORE(bytesToStore), CALL(MSTORE32); in: [bytesToStore, E: offset] out: [E: new offset]
1 :MSTORE(bytesToStore), CALL(MSTORE32); in: [bytesToStore, E: offset] out: [E: new offset]

; prepare return data
0 :MSTORE(retDataOffset)
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"devDependencies": {
"@0xpolygonhermez/zkevm-commonjs": "github:0xPolygonHermez/zkevm-commonjs#v9.0.0-rc.2-fork.13",
"@0xpolygonhermez/zkevm-proverjs": "github:0xPolygonHermez/zkevm-proverjs#develop-durian",
"@0xpolygonhermez/zkevm-testvectors": "github:0xPolygonHermez/zkevm-testvectors#v9.0.0-rc.2-fork.13",
"@0xpolygonhermez/zkevm-testvectors": "github:0xPolygonHermez/zkevm-testvectors#develop-durian",
"chai": "^4.3.6",
"chalk": "^3.0.0",
"eslint": "^8.25.0",
Expand Down
Loading

0 comments on commit 1a69e22

Please sign in to comment.