ADD-02 MitigationConfirmed

[c4-bot-1]

Lines of code

Vulnerability details

Mitigation of ADD-02: Mitigated with Error

Mitigated issue

L-02 in #507

The issue was a missing check for ECDSA signature malleability in Verification.verify().

Mitigation review - mitigated with error

Verification.sol has been overhauled and now uses OpenZeppelin's ECDSA library, which only allows s in the lower half order. The order of parameters of verify() was changed, and the corresponding changes have been made in calls to verify().

This mitigation includes the Error below.

Related to this mitigation is the mitigation of H-03 which added a signature check in FighterFarm.redeemMintPass(). This is also impacted by the Error of this mitigation.

Mitigation Error - toEthSignedMessageHash() is not in ECDSA.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 of Error

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

...

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

Remaining Low risk issue - no chainId in hash

The chainId is not included in the message hash, so cross-chain replay could be possible, if this were to be also deployed on other chains.

[c4-judge]

jhsagd76 marked the issue as satisfactory

[c4-judge]

jhsagd76 marked the issue as confirmed for report