code-423n4 / 2024-02-ai-arena-findings

4 stars 3 forks source link

Signature from `claimFighter()` can be re-used on other chains #716

Open c4-bot-8 opened 8 months ago

c4-bot-8 commented 8 months ago

Lines of code

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

Vulnerability details

Impact

Function claimFighter() enables users to claim a pre-determined number of fighters. This function verifies if the message signature is from the delegated address. However, the calculation of the signature does not take into a consideration block.chainid. This implies, that when smart contract will be deployed on the different chain - it will be possible to re-use the signature.

Proof of Concept

File: FighterFarm.sol

bytes32 msgHash = bytes32(keccak256(abi.encode(
            msg.sender, 
            numToMint[0], 
            numToMint[1],
            nftsClaimed[msg.sender][0],
            nftsClaimed[msg.sender][1]
        )));
        require(Verification.verify(msgHash, signature, _delegatedAddress));

As demonstrated above, msgHash does not use block.chaind to create a hash which is then verified.

This means that both signature and msgHash will be the same for different blockchains, thus it is possible to re-use signature on the different block.chainid.

Tools Used

Manual code review

Recommended Mitigation Steps

Make sure to include block.chainid into msgHash calculations. That way, msgHash will be different on different blockchains, thus the signature re-use won't be possible, becuase the msgHash on different blockchain wouldn't match the signature from another blockchain.

Assessed type

Other

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #56

c4-judge commented 8 months ago

HickupHH3 changed the severity to QA (Quality Assurance)