sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

bin2chen - leverageUpReceiver() Missing Security Check for msg_.marketHelper #93

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 5 months ago

bin2chen

high

leverageUpReceiver() Missing Security Check for msg_.marketHelper

Summary

In the TOFTMarketReceiverModule.leverageUpReceiver() function there is a lack of validation to verify whether the user-provided marketHelper is whitelisted. This oversight could allow malicious user to pass in a harmful marketHelper and construct arbitrary calls data.

Vulnerability Detail

The current implementation of TOFTMarketReceiverModule.leverageUpReceiver() is as follows:

    function leverageUpReceiver(bytes memory _data) public payable {
        /// @dev decode received message
        LeverageUpActionMsg memory msg_ = TOFTMsgCodec.decodeLeverageUpMsg(_data);

        /// @dev 'market'
@>      _checkWhitelistStatus(msg_.market);

        msg_.borrowAmount = _toLD(msg_.borrowAmount.toUint64());
        if (msg_.supplyAmount > 0) {
            msg_.supplyAmount = _toLD(msg_.supplyAmount.toUint64());
        }

        approve(address(msg_.market), type(uint256).max);

        {
@>          (Module[] memory modules, bytes[] memory calls) = IMarketHelper(msg_.marketHelper).buyCollateral(
                msg_.user, msg_.borrowAmount, msg_.supplyAmount, msg_.executorData
            );
            IMarket(msg_.market).execute(modules, calls, true);
        }

        approve(address(msg_.market), 0);

        emit LeverageUpReceived(msg_.user, msg_.market, msg_.borrowAmount, msg_.supplyAmount);
    }

As shown above, only the market validity is checked, but there is no validation for marketHelper. Consequently, users can provide a malicious marketHelper that returns harmful modules, calls, leading to the execution of IMarket(msg_.market).execute(modules, calls, true); not performing the expected buyCollateral() operation. For instance, it could execute unintended actions like borrow/removeCollateral.

Impact

The introduction of a malicious marketHelper could result in unexpected and dangerous operations, such as removeCollateral/borrow.

Code Snippet

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/modules/TOFTMarketReceiverModule.sol#L79

Tool used

Manual Review

Recommendation

    function leverageUpReceiver(bytes memory _data) public payable {
        /// @dev decode received message
        LeverageUpActionMsg memory msg_ = TOFTMsgCodec.decodeLeverageUpMsg(_data);

        /// @dev 'market'
        _checkWhitelistStatus(msg_.market);
+       _checkWhitelistStatus(msg_.marketHelper);

Duplicate of #90