Description

The final verification step in the PasskeyRegistryModule plug-in is to call the Verify() function in Secp256r1.sol. The latter does not adequately check the parameters before validation. The passKey parameter is picked directly from the internal mapping of public keys for msg.sender and is not directly controllable for someone else. Being of type uint, both r and s are guaranteed to be positive and the function also verifies that they are less than the order of the curve (the variable nn below). However, it is crucial to also verify that both r != 0 and s != 0 to avoid trivial signature bypasses.

function Verify(
    PassKeyId memory passKey,
    uint r,
    uint s,
    uint e
) internal view returns (bool) {
    if (r >= nn || s >= nn) {
        return false;
    }

    JPoint[16] memory points = _preComputeJacobianPoints(passKey);
    return VerifyWithPrecompute(points, r, s, e);
}

When the ECDSA verifies that a signature is signed by some public key, it takes in the tuple (r,s) (the signature pair) together with a public key and a hash. The hash is generated by hashing some representation of the operation that should be executed, and proving the signature for that hash means the owner of the public key approved the operation. The main calculation for verification in ECDSA is

R' = (h * s_inv) * G + (r * s_inv) * pubKey

where s_inv is the inverse of scalar s on the curve (i.e. inverse of s modulo the curve order) and h is the hash. The signature is said to be verified if the x-coordinate of the resulting point is equal to r, as in

(R').x == r

Replacing r and s with 0, we get that s_inv is also 0, and the calculation becomes

R' = (h * 0) * G + (0 * 0) * pubKey

R' = 0 * G + 0 * pubKey

R' = 0 == r

and the signature verification is always successful.

Impact

Anyone who can submit operations that will be validated by the PasskeyRegistryModule can impersonate other users and do anything that the account owner could do, leading to loss of funds.

Recommendations

Check that none of (r,s) are equal to zero.

function Verify(
    PassKeyId memory passKey,
    uint r,
    uint s,
    uint e
) internal view returns (bool) {
    if (r >= nn || s >= nn || r==0 || s==0) {
        return false;
    }

    JPoint[16] memory points = _preComputeJacobianPoints(passKey);
    return VerifyWithPrecompute(points, r, s, e);
}

Remediation

This issue has been acknowledged by Biconomy Labs, and a fix was implemented in commit 5c5a6bfe↗.