code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/Balancer.sol#L290 https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/Balancer.sol#L326 https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/Balancer.sol#L315

Vulnerability details

Impact

        routerETH.swapETH(
            _dstChainId,
            _oft, //refund
            abi.encodePacked(connectedOFTs[_oft][_dstChainId].dstOft),
            _amount,
            _computeMinAmount(_amount, _slippage)
        );

        router.swap(
            _dstChainId,
            _srcPoolId,
            _dstPoolId,
            _oft, //refund,
            _amount,
            _computeMinAmount(_amount, _slippage),
            _lzTxParams,
            _lzTxParams.dstNativeAddr,
            "0x"
        );

During stargate router swap method calls, the refund address is set to _oft. In case of excess ETH amount or excess gas, it will refunded to that address. Although these methods are called by a specific operator (owner), it's logical to refund to msg.sender.

        IStargateRouter.lzTxObj memory _lzTxParams = IStargateRouterBase
            .lzTxObj({
                dstGasForCall: 0,
                dstNativeAmount: msg.value,
                dstNativeAddr: abi.encode(
                    connectedOFTs[_oft][_dstChainId].dstOft
                )
            });

In https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/Balancer.sol#L315, setting dstNativeAmount to msg.value is wrong considering fee required. If it's intended to send native to destination, recommend to setting proper gas for call.

Proof of Concept

https://stargateprotocol.gitbook.io/stargate/developers/how-to-swap https://stargateprotocol.gitbook.io/stargate/interfaces/evm-solidity-interfaces/istargaterouter.sol

Tools Used

Manual

Recommended Mitigation Steps

Consider design decisions and tune parameters.

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #590

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)