Update Collateral Digest does not conform to EIP-712

Summary

The Update Collateral Digest does not conform to EIP-712.

Vulnerability Detail

Per the Sherlock's contest page, it stated that the code is expected to comply with EIP-712. As such, any non-compliance to EIP-712 found during the contest is considered valid.

Q: Is the code/contract expected to comply with any EIPs? Are there specific assumptions around adhering to those EIPs that Watsons should be aware of? $M is an ERC20Extended token which is ERC20 token extended with EIP-2612 permits for signed approvals (via EIP-712 and with EIP-1271 and EIP-5267 compatibility), and extended with EIP-3009 transfer with authorization (via EIP-712).

Per the EIP-712, the digest to be signed must be in the following format:

keccak256("\x19\x01" ‖ domainSeparator ‖ hashStruct(message))

However, the update collateral digest used by the Minter Gateway does not conform to this requirement. The digest does not include the \x19\x01 and domain separator in the payload to be hashed.

https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L958

File: MinterGateway.sol
958:     function _getUpdateCollateralDigest(
959:         address minter_,
960:         uint256 collateral_,
961:         uint256[] calldata retrievalIds_,
962:         bytes32 metadataHash_,
963:         uint256 timestamp_
964:     ) internal view returns (bytes32) {
965:         return
966:              (
967:                 keccak256(
968:                     abi.encode(
969:                         UPDATE_COLLATERAL_TYPEHASH,
970:                         minter_,
971:                         collateral_,
972:                         keccak256(abi.encodePacked(retrievalIds_)),
973:                         metadataHash_,
974:                         timestamp_
975:                     )
976:                 )
977:             );
978:     }

Impact

The minter gateway relies on the validator signatures to update the user's collateral value. EIP-712 is a standard dictating how to structure and sign data on Ethereum and should be followed closely to ensure compatibility with the EVM ecosystem.

Code Snippet

https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L958

Tool used

Manual Review

Recommendation

Ensure that the digest adheres to the following EIP-712 format:

keccak256("\x19\x01" ‖ domainSeparator ‖ hashStruct(message))

Duplicate of #12

PierrickGT commented 5 months ago

Invalid issue.

The watson omitted the _getDigest() function in their code example, which is the one including \x19\x01 and the domain separator:

sherlock-admin4 commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

observing the Readme, this should be valid; medium(1)

nevillehuang commented 5 months ago

Invalid, agree with sponsor comments here

sherlock-audit / 2023-10-mzero-judging

xiaoming90 - Update Collateral Digest does not conform to EIP-712 #65