REDUNDANT ERC1155 OCEAN TOKEN BALANCE UPDATE OF THE `OceanAdapter` CONTRACT COULD LEAD TO DoS OF THE `Ocean._computeOutputAmount` TRANSACTION

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/OceanAdapter.sol#L67 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/OceanAdapter.sol#L75 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/ocean/Ocean.sol#L419-L429 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/ocean/Ocean.sol#L757-L762

Vulnerability details

Impact

The Ocean._computeOutputAmount function is used to compute the output amount of an output token when the input token and input token amount is given. The Ocean._computeOutputAmount function mutates the ERC1155 token ledger amounts for the primitives and also calls the IOceanPrimitive(primitive).computeOutputAmount function which in turn creates interactions for ERC20 token unwrap and wrap.

When the OceanAdapter.computeOutputAmount function calls the wrap and unwrap function on the erc20 tokens in the Ocean contract the ERC1155 tokens are minted to the OceanAdapter contract address inside the Ocean._doInteraction function. But the Ocean ERC1155 tokens should represent the asset transfer which happens between the Ocean contract and the primitive contract only. There is no requirement to mint or burn the Ocean ERC1155 tokens since no asset transfer happens between the Ocean contract and the OceanAdapter contract.

At the end of the computeOutputAmount interaction the user balances of the ERC1155 tokens are also correctly updated.

The issue arises due to the unneccasary update of the ERC1155 token balances of the OceanAdapter contract during the intermediate wrap and unwrap of erc20 tokens happen during the computeOutputAmount transaction. The issue here is that if the OceanAdapter does not have a sufficient enough Ocean ERC1155 token balance then the computeOutputAmount will revert while trying to burn tokens (during unwrapERC20) from its balance due to integer underflow.

Proof of Concept

        unwrapToken(inputToken, inputAmount);

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/OceanAdapter.sol#L67

        wrapToken(outputToken, outputAmount);

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/OceanAdapter.sol#L75

        if (inputAmount > 0) {
            // since uint, same as (inputAmount != 0)
            _burn(userAddress, inputToken, inputAmount);
        }

        // if _executeInteraction returned a positive value for outputAmount,
        // this amount must be credited to the user's Ocean balance
        if (outputAmount > 0) {
            // since uint, same as (outputAmount != 0)
            _mint(userAddress, outputToken, outputAmount);
        }

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

        _increaseBalanceOfPrimitive(primitive, inputToken, inputAmount);

        outputAmount =
            IOceanPrimitive(primitive).computeOutputAmount(inputToken, outputToken, inputAmount, userAddress, metadata);

        _decreaseBalanceOfPrimitive(primitive, outputToken, outputAmount);

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

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended not to allow the Ocean._doInteraction function to mint or burn ERC1155 Ocean tokens from the OceanAdapter (from its children contracts to be precise) contract during the exeuction of the Ocean._computeOutputAmount transaction. This is because the ERC1155 token balnce update should only reflect the asset transfer between the Ocean contract and the primitive contract. Which means ERC1155 token balnce update should happen for the userAddress and the primitive address only.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

Inadequate elaboration and proof.

[c4-judge]

0xA5DF marked the issue as unsatisfactory: Insufficient proof