Lack of nonce in the UpdateCollateralDigest

Summary

The lack of nonce in the UpdateCollateralDigest allows attackers to replay the signatures to the gateway, allowing them to chain up this vulnerability with other vulnerabilities in the system to drain the Minter Gateway.

Vulnerability Detail

It is a standard practice to implement nonce to the digest to be signed to ensure that the signature can only be used once and to guard against potential replay attacks. However, it was observed that no nonce was included in the UpdateCollateralDigest.

Without considering other controls in places outside of the _verifyValidatorSignatures function, a signature from validators can be replayed and successfully verified by the _verifyValidatorSignatures function multiple times.

By chaining up this vulnerability, which allows signatures to be replayed, with other vulnerabilities raised in my other reports, such as the use of minimum timestamps instead of maximum timestamps, attackers can perform more sophisticated attacks that involve reorganizing and replaying signatures to drain the Minter Gateway. Refer to my "Using minimum timestamp across all signatures could drain the gateway" report for more details.

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:     }
..SNIP..
1045:     function _verifyValidatorSignatures(
1046:         address minter_,
1047:         uint256 collateral_,
1048:         uint256[] calldata retrievalIds_,
1049:         bytes32 metadataHash_,
1050:         address[] calldata validators_,
1051:         uint256[] calldata timestamps_,
1052:         bytes[] calldata signatures_
1053:     ) internal view returns (uint40 minTimestamp_) {
1054:         uint256 threshold_ = updateCollateralValidatorThreshold();
1055: 
1056:         minTimestamp_ = uint40(block.timestamp);
1057: 
1058:         // Stop processing if there are no more signatures or `threshold_` is reached.
1059:         for (uint256 index_; index_ < signatures_.length && threshold_ > 0; ++index_) {
..SNIP..
1074: 
1075:             // NOTE: Need to store the variable here to avoid a stack too deep error.
1076:             bytes32 digest_ = _getUpdateCollateralDigest(
1077:                 minter_,
1078:                 collateral_,
1079:                 retrievalIds_,
1080:                 metadataHash_,
1081:                 timestamps_[index_]
1082:             );

Impact

Code Snippet

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

Tool used

Manual Review

Recommendation

Consider including the nonce to the digest to prevent the signature from being replayed.

function _getUpdateCollateralDigest(
    address minter_,
    uint256 collateral_,
    uint256[] calldata retrievalIds_,
    bytes32 metadataHash_,
    uint256 timestamp_
) internal view returns (bytes32) {
    return
         (
            keccak256(
                abi.encode(
                    UPDATE_COLLATERAL_TYPEHASH,
                    minter_,
                    collateral_,
                    keccak256(abi.encodePacked(retrievalIds_)),
                    metadataHash_,
+                   minterUpdateNonces[minter_]++
                    timestamp_
                )
            )
        );
}

Duplicate of #46

sherlock-audit / 2023-10-mzero-judging

xiaoming90 - Lack of nonce in the UpdateCollateralDigest #61