_doMultiplenteractions can run out of gas due to undbounded loop

[c4-bot-10]

Lines of code

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

Vulnerability details

_doMultiplenteractions() can easily run out of gas and revert; costing user.

As an example; a user trying to 10 ERC20 WrapErc20 or UnwrapErc20 interactions will cost them over 1 million gas. Wrapping this many tokens is a likely scenario given the aim of ocean to connect with many Defi primitives; if say the user were holding multiple different tokens and planning on structuring many different transactions through Ocean.

The amount of interactions should therefore be limited so that users do not go over the block gas limit.

Impact

doMultipleInteractions() function can run out of gas when passed in large interactions array; reverting ans costing user the gas anyway. 20 wrap ERC20 token interactions will cost over 2 million in gas which can go over the block limit and cost the user significant money.

Proof of Concept

Add this test to TokenIntegration.js and run:

    describe("doMultipleInteractions ERC20wraps OOG", () => {
      let oceanIds;
      let numTokens;
      let WRAPPED_ETHER_ID;

      const decimals = "6";
      const mintAmount = shellV2.utils.numberWithFixedDecimals({ number: "100", decimals }); // 10e6

      before("Deploy token", async () => {
        oceanIds = [];
        numTokens = 20;
        interactions = [];
        WRAPPED_ETHER_ID = await ocean.WRAPPED_ETHER_ID();
        const erc20ContractFactory = await ethers.getContractFactory("ERC20MintsToDeployer");

        // Create numTokens amount of ERC20 Contracts and Wrap Interactions
        for (let i = 0; i < numTokens; i++) {
          // deploy erc20 token
          const token = await erc20ContractFactory.deploy(mintAmount, decimals);
          await token.deployed();

          // get OceanId for each token
          const oceanId = shellV2.utils.calculateWrappedTokenId({ address: token.address, id: 0 });

          // create Interactions
          const wrapInteraction = shellV2.interactions.wrapERC20({
            address: token.address,
            amount: mintAmount,
          });

          // approve ocean for each tokens mintAmount
          await token.connect(alice).approve(ocean.address, mintAmount);

          // confirm balances as expected
          const balanceAlice = await token.balanceOf(alice.address);
          expect(balanceAlice).to.equal(mintAmount);

          oceanIds.push(oceanId);
          interactions.push(wrapInteraction);
        }

      });

      it.only("Wrap multiple ERC20 & ETH", async () => {
        let responsePromise;

        responsePromise = await ocean
          .connect(alice)
          .doMultipleInteractions(interactions, oceanIds);

        const receipt = await (await responsePromise).wait(1);
        const gasUsed = receipt.gasUsed;
        const ethGasUsed = receipt.gasUsed.mul(receipt.effectiveGasPrice);

        expect(gasUsed).to.be.above(2000000);

        console.log("gasUsed", gasUsed);
        console.log("ethGasUsed", ethGasUsed.toString());
      });
    });

Tools Used

Hardhat Manual Review

Recommended Mitigation Steps

It is recommended to put a limit on the amount of interactions a user can have for WrapErc20 and UnwrapErc20 or for interactions as a whole; depending on expected use cases. See the check below for 15 interactions as an example.

    function _doMultipleInteractions(
        Interaction[] calldata interactions,
        uint256[] calldata ids,
        address userAddress
    )
        internal
        returns (
            uint256[] memory burnIds,
            uint256[] memory burnAmounts,
            uint256[] memory mintIds,
            uint256[] memory mintAmounts
        )
    {
+>      // Check if interactions array is greater than 15
+>      require(interactions.length <= 15, "Interactions exceed maximum limit");

        // Use the passed ids to create an array of balance deltas, used in
        // the intra-transaction accounting system.
        BalanceDelta[] memory balanceDeltas = new BalanceDelta[](ids.length);

        uint256 _idLength = ids.length;
        for (uint256 i = 0; i < _idLength;) {
            balanceDeltas[i] = BalanceDelta(ids[i], 0);
            unchecked {
                ++i;
            }
        }

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #3

[c4-judge]

0xA5DF changed the severity to QA (Quality Assurance)

[c4-judge]

0xA5DF marked the issue as grade-c

[0xA5DF]

Low quantity