Signature Malleability in Verification

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L206
https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/Verification.sol#L40

Vulnerability details

Impact

As highlighted in the EIP 865’s discussion, its current full implementation is affected by a signature malleability issue, steaming from the fact that in the current EIP, the recover function:

Allows both values 0/1 and 27/28 for v
Allows both lower and upper s values

ethereum/EIPs#865 (comment)

Signature malleability poses a security risk in systems that use these kinds of signatures as unique identifiers; as in the Verification.sol contract it uses ecrecover to recover the signer, this allows replay attacks, and this verification.verify is used in the FighterFarm.sol:
https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L206

https://swcregistry.io/docs/SWC-117/

Proof of Concept

    function verify(
        bytes32 msgHash, 
        bytes memory signature,
        address signer
    ) 
        public 
        pure 
        returns(bool) 
    {
        require(signature.length == 65, "invalid signature length");

        bytes32 r;
        bytes32 s;
        uint8 v;

        assembly {
            /*
            First 32 bytes stores the length of the signature

            add(sig, 32) = pointer of sig + 32
            effectively, skips first 32 bytes of signature

            mload(p) loads next 32 bytes starting at the memory address p into memory
            */

            // first 32 bytes, after the length prefix
            r := mload(add(signature, 32))
            // second 32 bytes
            s := mload(add(signature, 64))
            // final byte (first byte of the next 32 bytes)
            v := byte(0, mload(add(signature, 96)))
        }
        bytes memory prefix = "\x19Ethereum Signed Message:\n32";
        bytes32 prefixedHash = keccak256(abi.encodePacked(prefix, msgHash));
@>      return ecrecover(prefixedHash, v, r, s) == signer;
    }

Tools Used

Manual review.

Recommended Mitigation Steps

Consider using OpenZeppelin’s ECDSA library: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/ECDSA.sol

Assessed type

Other

[c4-bot-2]

Withdrawn by tpiliposian