[M-03] Lack of Public Key Validation in claimMessageMatchesSignature Function

[softstackio]

Description:
The claimMessageMatchesSignature function in the ClaimContract lacks crucial validation for the ECDSA public key coordinates (_pubKeyX and _pubKeyY) provided as input parameters. This omission could potentially allow the processing of invalid or maliciously crafted public keys, leading to security vulnerabilities in the claiming process.

Impact:

1. Security Vulnerability: Without proper validation, the contract may process claims using invalid public keys, potentially leading to unauthorized access to funds or manipulation of the claiming process.
2. Potential for Attacks: Malicious actors could exploit this lack of validation to submit carefully crafted invalid public keys that pass other checks but lead to unexpected behavior or vulnerabilities.
3. Integrity of Claiming Process: The absence of this check compromises the overall integrity and security of the claiming mechanism, as it relies on the validity of the public key for proper functionality.
4. Potential for Signature Forgery: In extreme cases, this vulnerability could allow an attacker to forge signatures, potentially claiming funds they are not entitled to.

The vulnerability is present in the `claimMessageMatchesSignature function:

function claimMessageMatchesSignature(
   address _claimToAddr,
   bytes memory _postFix,
   bytes32 _pubKeyX,
   bytes32 _pubKeyY,
   uint8 _v,
   bytes32 _r,
   bytes32 _s
) public view returns (bool) {
   // No validation of _pubKeyX and _pubKeyY
   // ...
}

The function accepts _pubKeyX and _pubKeyY without verifying if they represent a valid point on the secp256k1 curve used in Ethereum's ECDSA.

Proof of Concept:

An attacker could potentially pass invalid public key coordinates that might bypass other checks but lead to unexpected behavior:

1. Generate an invalid public key pair (X, Y) that doesn't lie on the secp256k1 curve.
2. Use this invalid key pair in a claim transaction.
3. The contract would process this claim without detecting the invalid key, potentially
   leading to security issues.

Recommendation:

1. Implement a validation function for the public key coordinates and use it in the claimMessageMatchesSignature function:
   error InvalidPublicKey();

2. Implement a validation function:

function isValidPublicKey(bytes32 _pubKeyX, bytes32 _pubKeyY) internal
pure returns (bool) {
   uint256 p =
0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F;
   uint256 x = uint256(_pubKeyX);
   uint256 y = uint256(_pubKeyY);
   if (x == 0 || x >= p || y == 0 || y >= p) {
       return false;
}
   // Check if the point is on the curve: y^2 = x^3 + 7 (mod p)
   uint256 lhs = mulmod(y, y, p);
   uint256 rhs = addmod(mulmod(mulmod(x, x, p), x, p), 7, p);
   return lhs == rhs;
}

3. Modify the claimMessageMatchesSignature function to use this validation:

function claimMessageMatchesSignature(
   address _claimToAddr,
   bytes memory _postFix,
   bytes32 _pubKeyX,
   bytes32 _pubKeyY,
   uint8 _v,
   bytes32 _r,
   bytes32 _s
) public view returns (bool) {
   if (!isValidPublicKey(_pubKeyX, _pubKeyY)) {
       revert InvalidPublicKey();
   }
   // Rest of the function...
}

[SurfingNerd]

further resources on that topic:
https://crypto.stackexchange.com/questions/105662/why-must-ecdsa-verification-ensure-the-point-is-on-the-curve