hats-finance / Safe-0x2909fdefd24a1ced675cb1444918fa766d76bdac

A collection of modules that can be used with the Safe contract
GNU Lesser General Public License v3.0
0 stars 1 forks source link

_N_DIV_2 value doesn't sync with EIP #10

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xc3b9eaf1decf7cca8ed2aff0e1037684dc6cf3f8b0a524c1781856d5c25d336e Severity: medium

Description: Description

_N_DIV_2 is not the same value which is described in eip1271 so it could give unexpected result

https://eips.ethereum.org/EIPS/eip-1271

    if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
      revert("SignatureValidator#recoverSigner: invalid signature 's' value");
    }

which is different than the used range in current implementation

nlordell commented 1 week ago

_N_DIV_2 is specific to the curve.

Signature malleability in ECC means that given a valid signature (r, s) for some message m, then (r, -s) is also valid for message m. This means that you cannot rely on the uniqueness of (r, s) in certain protocols (not used by the WebAuthn verifier, since signature uniqueness is not important for the application).

One way to prevent this is to ensure that s is in the "bottom half" of the curve, i.e. in the range (0, n/2) where n is the order of the curve.

Since this code is for the P256 curve, which has a curve order n of 0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551:

python3 -c "print(0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551 // 2)"
57896044605178124381348723474703786764998477612067880171211129530534256022184

Which is the value from the contracts.

The value you linked above is for the secp256k1 curve with a curve order of 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141:

$ python3 -c "print(hex(0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141 // 2))"
0x7fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a0