c4-bot-7 commented 5 months ago

Lines of code

https://github.com/ArenaX-Labs/2024-02-ai-arena-mitigation/blob/d81beee0df9c5465fe3ae954ce41300a9dd60b7f/src/Verification.sol#L20

Vulnerability details

Impact

Verification.verify() reverts. This breaks AAMintPass.claimMintPass(), FighterFarm.claimFighters() and FighterFarm.redeemMintPass().

Proof of Concept

Verification.sol#L20:

bytes32 ethSignedMessageHash = messageHash.toEthSignedMessageHash();

will revert because toEthSignedMessageHash() is located in MessageHashUtils.sol.

This breaks AAMintPass.claimMintPass(), FighterFarm.claimFighters() and FighterFarm.redeemMintPass(), which call Verification.verify().

Recommended Mitigation Steps

import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
+ import "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";

...

- bytes32 ethSignedMessageHash = messageHash.toEthSignedMessageHash();
+ bytes32 ethSignedMessageHash = MessageHashUtils.toEthSignedMessageHash(messageHash);

Assessed type

Library

fnanni-0 commented 5 months ago

This analyzes openzeppelin’s v5.0.0 ECDSA contract for solidity versions ^0.8.20. However AI Arena contracts are compiled with solidity version 0.8.13. From what I understand, we should look at ECSDA contract from the v4.7 release https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.7/contracts/utils/cryptography/ECDSA.sol, which includes the toEthSignedMessageHash function.

c4-judge commented 5 months ago

jhsagd76 marked the issue as unsatisfactory: Invalid

d3e4 commented 5 months ago

This analyzes openzeppelin’s v5.0.0 ECDSA contract for solidity versions ^0.8.20. However AI Arena contracts are compiled with solidity version 0.8.13. From what I understand, we should look at ECSDA contract from the v4.7 release https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.7/contracts/utils/cryptography/ECDSA.sol, which includes the toEthSignedMessageHash function.

@fnanni @jhsagd76 How does this actually work? Where is the lock to version v4.7? import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; will import the latest version which requires 0.8.20. Verification.sol does not restrict the compiler version.

fnanni-0 commented 5 months ago

@fnanni @jhsagd76 How does this actually work? Where is the lock to version v4.7? import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; will import the latest version which requires 0.8.20. Verification.sol does not restrict the compiler version.

I don't think it's possible to compile one part of a contract with one compiler version and another part with another version. Verification.sol gets compiled with solidity version 0.8.13 as all AI Arena contracts (see the foundry.toml file). Therefore, it shouldn't be possible to inherit contracts with pragma solidity ^0.8.20.

Openzeppelin contracts at commit @ ecd2ca2 are used.