Skip to content

Commit

Permalink
Fix rejecting public keys of the form (0, y) (#16)
Browse files Browse the repository at this point in the history
* Fix rejecting public keys of the form (0, y)

Veridise Audit ID 10

* deploy contract
  • Loading branch information
nalinbhardwaj authored Sep 25, 2023
1 parent e450bca commit 301328c
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 38 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ The secp256r1 elliptic curve, aka P256, is interesting because it's supported by

## Usage

**Address `0xea923BEe7108728eA2708af25e9981272193a555`**
**Address `0x86e49A916721C4542CD1378D43c9f5C7B501de81`**

Available on any chain. If missing, see `deploy.sh`.

Expand All @@ -19,7 +19,7 @@ bytes32 hash; // message hash
uint256 r, s; // signature
uint256 x, y; // public key
address verifier = 0xea923BEe7108728eA2708af25e9981272193a555;
address verifier = 0x86e49A916721C4542CD1378D43c9f5C7B501de81;
bytes memory args = abi.encode(hash, r, s, x, y);
(bool success, bytes memory ret) = verifier.staticcall(args);
assert(success); // never reverts, always returns 0 or 1
Expand Down
26 changes: 13 additions & 13 deletions broadcast/Deploy.s.sol/84531/run-latest.json

Large diffs are not rendered by default.

18 changes: 9 additions & 9 deletions lcov.info
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ DA:97,2262
FN:104,P256Verifier.ecAff_isOnCurve
FNDA:2269,P256Verifier.ecAff_isOnCurve
DA:108,2269
BRDA:108,4,0,2265
BRDA:108,4,1,4
DA:109,4
DA:112,2265
DA:113,2265
DA:114,2265
DA:116,2265
BRDA:108,4,0,2267
BRDA:108,4,1,1
DA:109,2
DA:112,2267
DA:113,2267
DA:114,2267
DA:116,2267
FN:124,P256Verifier.ecZZ_mulmuladd
FNDA:2262,P256Verifier.ecZZ_mulmuladd
DA:130,2262
Expand Down Expand Up @@ -114,8 +114,8 @@ BRDA:216,15,0,2262
BRDA:216,15,1,-
DA:218,2262
DA:220,2262
FN:227,P256Verifier.ecAff_IsZero
FNDA:4524,P256Verifier.ecAff_IsZero
FN:227,P256Verifier.ecAff_IsInf
FNDA:4524,P256Verifier.ecAff_IsInf
DA:233,4524
FN:243,P256Verifier.ecZZ_dadd_affine
FNDA:430879,P256Verifier.ecZZ_dadd_affine
Expand Down
4 changes: 2 additions & 2 deletions script/deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
forge script DeployScript --via-ir --optimizer-runs=999999 --rpc-url $RPC_URL --broadcast --private-key $PRIVATE_KEY

# Update contract address
ADDR=0xea923BEe7108728eA2708af25e9981272193a555
ADDR=0x86e49A916721C4542CD1378D43c9f5C7B501de81

# Verify to Etherscan
forge verify-contract $ADDR P256Verifier --optimizer-runs=999999 --constructor-args "0x" --show-standard-json-input > script/etherscan.json
Expand All @@ -13,4 +13,4 @@ forge verify-contract $ADDR P256Verifier --optimizer-runs=999999 --constructor-a
# Finally, manually verify to Etherscan

# Success
# https://goerli.basescan.org/address/0xea923BEe7108728eA2708af25e9981272193a555#code
# https://goerli.basescan.org/address/0x86e49A916721C4542CD1378D43c9f5C7B501de81#code
4 changes: 2 additions & 2 deletions script/etherscan.json

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions src/P256Verifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ contract P256Verifier {
uint256 x,
uint256 y
) internal pure returns (bool) {
if (0 == x || x >= p || 0 == y || y >= p) {
if (x >= p || y >= p || (x == 0 && y == 0)) {
return false;
}

Expand Down Expand Up @@ -212,25 +212,25 @@ contract P256Verifier {
uint256 zz1;
uint256 zzz1;

if (ecAff_IsZero(x1, y1)) return (x2, y2, true);
if (ecAff_IsZero(x2, y2)) return (x1, y1, true);
if (ecAff_IsInf(x1, y1)) return (x2, y2, true);
if (ecAff_IsInf(x2, y2)) return (x1, y1, true);

(x1, y1, zz1, zzz1) = ecZZ_dadd_affine(x1, y1, 1, 1, x2, y2);

return ecZZ_SetAff(x1, y1, zz1, zzz1);
}

/**
* @dev Check if the curve is the zero curve in affine rep
* Assumes point is on the EC or is the zero point.
* @dev Check if a point is the infinity point in affine rep.
* Assumes point is on the EC or is the point at infinity.
*/
function ecAff_IsZero(
uint256,
function ecAff_IsInf(
uint256 x,
uint256 y
) internal pure returns (bool flag) {
// invariant((x == 0 && y == 0) || ecAff_isOnCurve(x, y));

return (y == 0);
return (x == 0 && y == 0);
}

/**
Expand Down
3 changes: 1 addition & 2 deletions test/P256Verifier.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,10 @@ contract P256VerifierTest is Test {
assertEq(result, false);
assertLt(gasUsed, 1500);

// p-1 is in-bounds, takes more gas again.
// p-1 is in-bounds but point is not on curve.
(x, y) = (p - 1, 1);
(result, gasUsed) = evaluate(hash, r, s, x, y);
console2.log("gasUsed ", gasUsed);
assertEq(result, false);
assertGt(gasUsed, 1500);
}
}

0 comments on commit 301328c

Please sign in to comment.