sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

duc - The mismatch between leverage executor contracts and the utilized interface in market #62

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

duc

medium

The mismatch between leverage executor contracts and the utilized interface in market

Summary

Each market (BigBang or Singularity) has a leverage executor contract to swap assets or collateral during leverage actions by calling the getCollateral() or getAsset() functions. However, the actual implementations of these functions in the leverage executor contract mismatch with the integration and interface in the market.

Vulnerability Detail

In BBLeverage.buyCollateral() function, it call leverageExecutor.getCollateral() with 6 parameters.

amountOut = leverageExecutor.getCollateral(
    collateralId,
    address(asset),
    address(collateral),
    memoryData.supplyShareToAmount + memoryData.borrowShareToAmount,
    calldata_.from,
    calldata_.data
);

leverageExecutor is declared in Market contract as ILeverageExecutor from tapioca-periph/interfaces/bar/ILeverageExecutor.sol: Market.sol#L10, Market.sol#L108

However, the actual implementation of the leverage executor contract is different, with only 4 parameters in the getCollateral() function. All of the SimpleLeverageExecutor, AssetToSGLPLeverageExecutor, and AssetToTSDaiLeverageExecutor contracts inherit from the BaseLeverageExecutor contract and have the getCollateral() function with only 4 parameters.

In BaseLeverageExecutor contract:

function getCollateral(address assetAddress, address collateralAddress, uint256 assetAmountIn, bytes calldata data)
        external
        payable
        virtual
        returns (uint256 collateralAmountOut)
    {}

In SimpleLeverageExecutor contract:

function getCollateral(
    address assetAddress,
    address collateralAddress,
    uint256 assetAmountIn,
    bytes calldata swapperData
) external payable override returns (uint256 collateralAmountOut) {
    // Should be called only by approved SGL/BB markets.
    if (!cluster.isWhitelisted(0, msg.sender)) revert SenderNotValid();
    return _swapAndTransferToSender(true, assetAddress, collateralAddress, assetAmountIn, swapperData);
}

Therefore, those contracts are mismatch with the interface and external calls in the market, resulting in unable to use them. The same vulnerability exist for getAsset() function.

Impact

Leverage executor contracts will be unable to be used for leverage functionality (breaking core functionality)

Code Snippet

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/singularity/SGLLeverage.sol#L77-L84 https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/leverage/BaseLeverageExecutor.sol#L95-L100 https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/leverage/SimpleLeverageExecutor.sol#L38-L47

Tool used

Manual Review

Recommendation

Should fix the contract implementation to match the interface used in the Market.

Duplicate of #115

sherlock-admin3 commented 5 months ago

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

WangAudit commented:

BaseLeverageExecutor doesn't inherit ILeverageExecutor. If we look at Market.sol (this is where we declare leverageExecutor) it's ILeverageExecutor type. And this interface indeed has assetId as an input parameter -> BaseLeverageExecutor will not be called here cause BaseLeverageExecutor doesn't inherit ILeverageExecutor