FastTiger - The mint and burn function is vulnerable to Sandwitch Attack at `Minter Rate` update.

[sherlock-admin3]

FastTiger

high

The mint and burn function is vulnerable to Sandwitch Attack at Minter Rate update.

Summary

If an attacker finds a proposal in the mempool that reduces the Minter Rate, he or she can retain MToken for free through a Sandwitch attack.

Vulnerability Detail

The MinterGateway.sol#_rate function and MinterRateModel.sol#rate function are as follows.

    function _rate() internal view override returns (uint32 rate_) {
        (bool success_, bytes memory returnData_) = rateModel().staticcall(
            abi.encodeWithSelector(IRateModel.rate.selector)
        );

        rate_ = (success_ && returnData_.length >= 32) ? UIntMath.bound32(abi.decode(returnData_, (uint256))) : 0;
    }

    function rate() external view returns (uint256 rate_) {
        return uint256(ITTGRegistrar(ttgRegistrar).get(_BASE_MINTER_RATE));
    }

As shown on the right, rate_ recalled in the MinterGateway.sol#_rate function is the Minter Rate value currently set in TTG. Additionally, the MinterGateway.sol#currentIndex function is as follows.

    function currentIndex() public view override(ContinuousIndexing, IContinuousIndexing) returns (uint128) {
        // NOTE: Safe to use unchecked here, since `block.timestamp` is always greater than `latestUpdateTimestamp`.
        unchecked {
            return
                // NOTE: Cap the index to `type(uint128).max` to prevent overflow in present value math.
                UIntMath.bound128(
                    ContinuousIndexingMath.multiplyIndicesUp(
                        latestIndex,
                        ContinuousIndexingMath.getContinuousIndex(
                            ContinuousIndexingMath.convertFromBasisPoints(_latestRate),
                            uint32(block.timestamp - latestUpdateTimestamp)
                        )
                    )
                );
        }
    }

As shown on the right, the currentIndex value changes exponentially in proportion to _latestRate. The attacker uses the above facts to carry out the following attack when the Minter Rate becomes small.

1. Call mintM() when Minter Rate is high. For example, let's say currentIndex=5. At this time, if you mint 1000 MToken, PrincipalAmount increases by 1000/5 = 200.
2. ‘Minter Rate’ has decreased.
3. The attacker calls ContinuousIndexing.sol#updateIndex to change the currentIndex value. For example, let's decrement and say currentIndex = 4. 4.At this time, the attacker calls burnM(). At this time, since the Principal Amount is 200, only 200 * 4 = 800 MToken are needed. Therefore, the attacker benefited from 200 MToken.

   Impact

   An attacker can receive MToken without any compensation.

   Code Snippet

   https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L261 https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L322 https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L981-L984 https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/rateModels/MinterRateModel.sol#L35-L37 https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L683 https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L981-L984

   Tool used

Manual Review

Recommendation

Implement pause(), unpause() mechanism when the TTG updates Minter Rate.

[deluca-mike]

This is an invalid issue since the current index (and even latest index, for that matter) are always increasing.

Trying to create a PoC would have revealed that this is not a possible attack or state change.

[nevillehuang]

Request poc

[sherlock-admin3]

PoC requested from @FastTiger777

Requests remaining: 3

[FastTiger777]

contract Test is MinterGatewayTests { function test_sandwithch(){ uint256 amount = 80e18; uint48 mintId = 1;

    _minterGateway.setCollateralOf(_minter1, 100e18);
    _minterGateway.setUpdateTimestampOf(_minter1, block.timestamp);

    _minterGateway.setMintProposalOf(_minter1, mintId, amount, block.timestamp, _alice);

    _mToken.setIsEarning(_minter1, true);

    vm.warp(block.timestamp + _mintDelay);

    vm.prank(_minter1);

    _minterRateModel.setRate(1000);

    (,uint256 amount_mintM) = _minterGateway.mintM(mintId);
    console.log("Mint amount: " + amount_mintM.toString());

    _minterRateModel.setRate(400);

    (,uint256 amount_burnM) = _minterGateway.burnM(_minter1, _getPrincipalAmountRoundedDown(UIntMath.safe240(amount_mintM)),
                                amount_mintM);
    console.log("Burn amount: " + amount_burnM.toString());
}

}

[deluca-mike]

I don't know what that PoC is showing or doing, but I fixed it up so that it can compile and run at all, and:

contract Test is MinterGatewayTests {
    function test_sandwich() external {
        uint256 amount = 80e18;
        uint48 mintId = 1;

        console2.log("Current Index:", _minterGateway.currentIndex()); // 1000000000000

        _minterGateway.setCollateralOf(_minter1, 100e18);
        _minterGateway.setUpdateTimestampOf(_minter1, block.timestamp);
        _minterGateway.setMintProposalOf(_minter1, mintId, amount, block.timestamp, _alice);

        vm.warp(block.timestamp + _mintDelay);

        console2.log("Current Index:", _minterGateway.currentIndex()); // 1000001268391

        _minterRateModel.setRate(1000);

        console2.log("Current Index:", _minterGateway.currentIndex()); // 1000001268391

        vm.prank(_minter1);
        (, uint256 amount_mintM) = _minterGateway.mintM(mintId);

        console2.log("Current Index:", _minterGateway.currentIndex()); // 1000001268391

        _minterRateModel.setRate(400);

        console2.log("Current Index:", _minterGateway.currentIndex()); // 1000001268391

        (, uint256 amount_burnM) = _minterGateway.burnM(_minter1, uint240(amount_mintM));

        console2.log("Current Index:", _minterGateway.currentIndex()); // 1000001268391
    }
}

So the provided PoC is invalid.

To be clear, I don't even need a PoC of the attack, I just need a PoC that shows that currentIndex ever decreases.

[FastTiger777]

sorry, this is invalid