Using ETH in doMultipleTransactions will cause the transaction to revert even though it is payable

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/ocean/Ocean.sol#L474-L541

Vulnerability details

Impact

Users may choose to use the doMultipleTransactions in order to chain together multiple interactions with the protocol however, because ETH has no decimals function, the function will revert mid transaction. This could especially be impactful if external automated applications rely on the Ocean contract resulting in a case where ETH tokens attempting to be wrapped along with other complicated strategies in the form of interactions could revert.

Proof of Concept

The proof of concept below outlines this scenario:

pragma solidity 0.8.20;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "../ocean/Interactions.sol";
import "../ocean/Ocean.sol";
import "../adapters/Curve2PoolAdapter.sol";

import "forge-std/Test.sol";
import "forge-std/console.sol";

import "./mocks/MockERC20.sol";

contract MyTests is Test {

    address deployer = vm.addr(1);
    address alice = vm.addr(2);
    address bob = vm.addr(3);
    address eve = vm.addr(4);

    Ocean ocean;
    Curve2PoolAdapter adapter;
    address ethAddress = address(0x4574686572);
    uint256 ethId = _calculateOceanId(address(0x4574686572));

    address curve2PoolAddr = 0x7f90122BF0700F9E7e1F688fe926940E8839F353;

    IERC20 usdc = IERC20(0xFF970A61A04b1cA14834A43f5dE4533eBDDB5CC8);
    IERC20 usdt = IERC20(0xFd086bC7CD5C481DCC9C85ebE478A1C0b69FCbb9);
    address wallet = 0x9b64203878F24eB0CDF55c8c6fA7D08Ba0cF77E5; // USDC/USDT whale
    address lpWallet = 0x641D99580f6cf034e1734287A9E8DaE4356641cA; // 2pool LP whale

    function setUp() public {
        vm.createSelectFork("https://arb-mainnet.g.alchemy.com/v2/DsRZFUFhL-SizQtsqUusTTnvVN9eguvI");
        vm.startPrank(deployer);
        ocean = new Ocean("");
        adapter = new Curve2PoolAdapter(address(ocean), curve2PoolAddr);

        vm.stopPrank();

    }

    function testEthRevert() public {
        // Setup scenario
        vm.deal(address(this), 100 ether);
        deal(address(usdt), address(this), 10_000 * 1e6);
        deal(address(usdc), address(this), 10_000 * 1e6);

        usdc.approve(address(ocean), type(uint256).max);
        usdt.approve(address(ocean), type(uint256).max);

        Interaction[] memory inters = new Interaction[](3);
        inters[0] = Interaction({
            interactionTypeAndAddress: _fetchInteractionId(address(ethAddress), 0),
            inputToken: 0,
            outputToken: 0,
            specifiedAmount: 0,
            metadata: ""
        });
        inters[1] = Interaction({
            interactionTypeAndAddress: _fetchInteractionId(address(usdc), 0),
            inputToken: 0,
            outputToken: 0,
            specifiedAmount: 10_000 ether,
            metadata: ""
        });
        inters[2] = Interaction({
            interactionTypeAndAddress: _fetchInteractionId(address(usdt), 0),
            inputToken: 0,
            outputToken: 0,
            specifiedAmount: 10_000 ether,
            metadata: ""
        });

        uint256[] memory ids = new uint256[](3);
        ids[0] = _calculateOceanId(address(ethAddress));
        ids[1] = _calculateOceanId(address(usdc));
        ids[2] = _calculateOceanId(address(usdt));

                vm.expectRevert();
        ocean.doMultipleInteractions{value: 15 ether}(inters, ids);

        console.log(ocean.balanceOf(address(this), _calculateOceanId(address(ethAddress))));
        console.log(ocean.balanceOf(address(this), _calculateOceanId(address(usdc))));
        console.log(ocean.balanceOf(address(this), _calculateOceanId(address(usdt))));

    }

    function _fetchInteractionId(address token, uint256 interactionType) internal pure returns (bytes32) {
        uint256 packedValue = uint256(uint160(token));
        packedValue |= interactionType << 248;
        return bytes32(abi.encode(packedValue));
    }

    function _calculateOceanId(address tokenAddress) internal pure returns (uint256) {
        return uint256(keccak256(abi.encodePacked(tokenAddress, uint256(0))));
    }

    // Receivers
    function onERC1155Received(
        address operator,
        address from,
        uint256 id,
        uint256 value,
        bytes calldata data
    ) external returns (bytes4) {
        return bytes4(keccak256("onERC1155Received(address,address,uint256,uint256,bytes)"));
    }

    function onERC1155BatchReceived(
        address operator,
        address from,
        uint256[] calldata ids,
        uint256[] calldata values,
        bytes calldata data
    ) external returns (bytes4) {
        return bytes4(keccak256("onERC1155BatchReceived(address,address,uint256[],uint256[],bytes)"));
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        return bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"));
    }

}

Tools Used

Manual Review

Recommended Mitigation Steps

I recommend creating a special interaction execution method in _executeInteraction for wrapping ETH or handling ETH tokens appropriately in the _erc20Wrap function. Alternatively, the cause of wrapping ETH could also be handled in the for loop similarly to what is done in _doInteraction.

Assessed type

Error

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #35

[c4-judge]

0xA5DF marked the issue as unsatisfactory: Invalid