code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/cfed0dfa3bebdac0993b1b42239b4944eb0b196c/src/ulysses-omnichain/MulticallRootRouter.sol#L111-L126 https://github.com/code-423n4/2023-05-maia/blob/cfed0dfa3bebdac0993b1b42239b4944eb0b196c/src/ulysses-omnichain/MulticallRootRouter.sol#L137-L152

Vulnerability details

Impact

Some tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value. For example Tether (USDT)’s approve() function will revert if the current approval is not zero, to protect against front-running changes of approvals.

Link to usdt contract reference(SLOC 199-209)

approve is actually vulnerable to a sandwich attack as explained in the following document and this check for allowance doesn't actually avoid it.

Reference document link- https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit

In ERC20, front running attack is possible via approve() function,

Reference link for better understanding- https://blog.smartdec.net/erc20-approve-issue-in-simple-words-a41aaf47bca6

In the protocol, all functions using approve() must be first approved by zero. The _approveAndCallOut() and _approveMultipleAndCallOut() are called to make ERC20 approvals. But it does not approve 0 first.

File: src/ulysses-omnichain/MulticallRootRouter.sol

    function _approveAndCallOut(
        address owner,
        address recipient,
        address outputToken,
        uint256 amountOut,
        uint256 depositOut,
        uint24 toChain
    ) internal virtual {
        //Approve Root Port to spend/send output hTokens.
        ERC20hTokenRoot(outputToken).approve(bridgeAgentAddress, amountOut);

        // some code

File: src/ulysses-omnichain/MulticallRootRouter.sol

    function _approveMultipleAndCallOut(
        address owner,
        address recipient,
        address[] memory outputTokens,
        uint256[] memory amountsOut,
        uint256[] memory depositsOut,
        uint24 toChain
    ) internal virtual {
        //For each output token
        for (uint256 i = 0; i < outputTokens.length;) {
            //Approve Root Port to spend output hTokens.
            ERC20hTokenRoot(outputTokens[i]).approve(bridgeAgentAddress, amountsOut[i]);

        // some code