sherlock-audit / 2024-02-tapioca-judging

3 stars 2 forks source link

GiuseppeDeLaZara - StargateRouter cannot send payloads and rebalancing of ERC20s is broken #68

Open sherlock-admin2 opened 3 months ago

sherlock-admin2 commented 3 months ago

GiuseppeDeLaZara

high

StargateRouter cannot send payloads and rebalancing of ERC20s is broken

Summary

The Balancer.sol contract can't perform the rebalancing of ERC20s across chains as the Stargate router is not able to send any payload and will immediately revert the transaction if a payload is included. In this instance payload is hardcoded to "0x".

Vulnerability Detail

Balancer.sol contract has a rebalance function that is supposed to perform a rebalancing of mTOFTs across chains. In case the token being transferred through Stargate is an ERC20 it is using the Stargate router to initiate the transfer. The issue however is that the stargate router is not able to send any payload and will immediately revert the transaction if a payload is included.

If we take a look at the code, there is a payload equal to "0x" being sent with the transaction:

## Balancer.sol

    router.swap{value: msg.value}(
        _dstChainId,
        _srcPoolId,
        _dstPoolId,
        payable(this),
        _amount,
        _computeMinAmount(_amount, _slippage),
        IStargateRouterBase.lzTxObj({dstGasForCall: 0, dstNativeAmount: 0, dstNativeAddr: "0x0"}),
        _dst,
>>>>    "0x" => this is the payload that is being sent with the transaction
    );

As a proof of concept we can try to send a payload through the stargate router on a forked network and see that the transaction will revert. p.s. make sure to run on it on a forked network on Ethereum mainnet.

function testStargateRouterReverting() public {
    vm.createSelectFork(vm.envString("MAINNET_RPC_URL"));

    address stargateRouter = 0x8731d54E9D02c286767d56ac03e8037C07e01e98;
    address DAIWhale = 0x7A8EDc710dDEAdDDB0B539DE83F3a306A621E823;
    address DAI = 0x6B175474E89094C44Da98b954EedeAC495271d0F;
    IStargateRouter.lzTxObj memory lzTxParams = IStargateRouter.lzTxObj(0, 0, "0x00");

    vm.startPrank(DAIWhale);
    vm.deal(DAIWhale, 5 ether);
    IERC20(DAI).approve(stargateRouter, 1e18);
    IStargateRouter(stargateRouter).swap{value: 1 ether}(
        111, 3, 3, payable(address(this)), 1e18, 1, lzTxParams, abi.encode(address(this)), "0x"
    );
}

It fails with the following error:

Screenshot 2024-03-11 at 11 52 31

Proof of concept was tested on Ethereum network, but it applies to all the other blockchains as well.

By looking at the Stargate documentation we can see that it is highlighted to use the StargateComposer instead of the StargateRouter if sending payloads: https://stargateprotocol.gitbook.io/stargate/stargate-composability.

Both StargateRouter and StargateComposer have the swap interface, but the intention was to use the StargateRouter which can be observed by the retryRevert function in the Balancer.sol contract.

## Balancer.sol

function retryRevert(uint16 _srcChainId, bytes calldata _srcAddress, uint256 _nonce) external payable onlyOwner {
    router.retryRevert{value: msg.value}(_srcChainId, _srcAddress, _nonce);
}

StargateComposer does not have the retryRevert function. Its code be found here: https://www.codeslaw.app/contracts/ethereum/0xeCc19E177d24551aA7ed6Bc6FE566eCa726CC8a9.

As this makes the rebalancing of mTOFTs broken, I'm marking this as a high-severity issue.

Impact

Rebalancing of mTOFTs across chains is broken and as it is one of the main functionalities of the protocol, this is a high-severity issue.

Code Snippet

Tool used

Manual Review

Recommendation

Use the StargateComposer instead of the StargateRouter if sending payloads.

cryptotechmaker commented 3 months ago

Invalid; Duplicate of https://github.com/sherlock-audit/2024-02-tapioca-judging/issues/69

windhustler commented 3 months ago

For sending ERC20s with Stargate you need to use the StargateComposer contract and not the StargateRouter. As StargateComposer doesn't have the retryRevert function you should remove it from the Balancer.sol.

nevillehuang commented 3 months ago

@0xRektora @cryptotechmaker Might want to take a look, but seems like the same underlying root cause related to configuration of stargaterouter. I checked the composer contract and I believe @windhustler is right. I am also inclined to think they are not duplicates. Let me know if I am missing something.

cryptotechmaker commented 3 months ago

@nevillehuang It's duplicate in the sense that #69 mentioned an issue that's being fixed by using StargateComposer, which is the same solution for this one

Please lmk if otherwise

nevillehuang commented 3 months ago

Hi @cryptotechmaker consulted tapioca's internal judge @cvetanovv and agree although fixes are similar, different funcitonalities are impacted and so it can be seen as two separate fixes combined into one, so will be separating this from #69

cryptotechmaker commented 2 months ago

@nevillehuang Sure! However, there's not going to be any PR for the issue as we plan to use StargateComposer