The 'r' Value of A Signature Should Be Within (0, n) Where 'n' is the Subgroup Order

[hats-bug-reporter[bot]]

Github username: @MatinR1 Twitter username: MatinRezaii1 Submission hash (on-chain): 0xc634ce8be8e2d7c9d534a144da53a46d3d332bfba64fdd564f9c4d9464b4ce19 Severity: high

Description: Description\ secp256r1 is a 256-bit prime field Weierstrass curve with these data:

$$ y^2 = x^3 + ax + b $$

Name	Value
p	0xffffffff00000001000000000000000000000000ffffffffffffffffffffffff
a	0xffffffff00000001000000000000000000000000fffffffffffffffffffffffc
b	0x5ac635d8aa3a93e7b3ebbd55769886bc651d06b0cc53b0f63bce3c3e27d2604b
G	(0x6b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c296, 0x4fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f5)
n	0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551
h	0x1

The main difference between secp256k1 and secp256r1 is that secp256k1 is a Koblitz curve, while secp256r1 is a prime field curve. Koblitz curves are generally known to be a few bits weaker than prime field curves, but when talking about 256-bit curves, it has little impact. For these kinds of curves, the EIP-7212 is proposed which gives some strong considerations to ensure that any precompiled secp256r1 contract works properly.

These considerations are:

1. Verify that the r and s values are in (0, n) (exclusive) where n is the order of the subgroup.
2. Verify that the point formed by (x, y) is on the curve and that both x and y are in [0, p) (inclusive 0, exclusive p) where p is the prime field modulus. Note that many implementations use (0, 0) as the reference point at infinity, which is not on the curve and should therefore be rejected.

Also, as these curves allow signature malleability by their nature, (e.g. the EIP-2 allows symmetrical s values, +s or -s, to be passed on another incorrect point on secp256k1 curve and the result is considered to be valid) it is always checked that the s value should be less than half of the curve subgroup order (the n parameter above).

Thus, we can infer that these checks should be presented as these:

1

$$ 0 < s < \frac{n}{2} $$

2

$$ 0 < r < n $$

Also if we look at the Python implementation of EVM, we can see that the ecrecover() function also checks this criteria:

def ecrecover(computation: ComputationAPI) -> ComputationAPI:

...
    try:
        validate_lt_secpk1n(r, title="ECRecover: R")
        validate_lt_secpk1n(s, title="ECRecover: S")

The bounds for the s value is correctly checked inside the P256 library, however, if we look at the P256 library, we cannot see such a check for the r parameter. This will lead to large numbers for r considered to be valid and passed to the function verifySignature().

Attack Scenario\

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional) Consider checking the bounds of the r parameter of a message to not deviate the EIP-7212 standard.

[0xEricTee]

The r value check is done in modules/passkey/contracts/verifiers/FCLP256Verifier.sol contract, specifically in the library ecdsa_verify::ecdsa_verify :

 if (r == 0 || r >= FCL_Elliptic_ZZ.n || s == 0 || s >= FCL_Elliptic_ZZ.n) {
            return false;
        }

[nlordell]

I would also like to add that from the EIP (emphasis my own):

Required Checks in Verification

The following requirements MUST be checked by the precompiled contract to verify signature components are valid:

• Verify that the r and s values are in (0, n) (exclusive) where n is the order of the subgroup.
• Verify that the point formed by (x, y) is on the curve and that both x and y are in [0, p) (inclusive 0, exclusive p) where p is the prime field modulus. Note that many implementations use (0, 0) as the reference point at infinity, which is not on the curve and should therefore be rejected.

Essentially the EIP specifies that the P256 verification implementation (so IP256Verifier, here implemented by FCLP256Verifier as pointed out by @0xEricTee) is responsible for checking the validity of r and not the caller as you suggest in the submission.

I also believe this to be invalid.