When InteractionType is ComputeOutputAmount, users have to pay the fee twice and this will lead to loss of user fund

[c4-bot-1]

Lines of code

https://github.com/Shell-Protocol/Shell-Protocol/blob/c1e3615130dcbbd307ce72e444290a327f8db69c/src/ocean/Ocean.sol#L694

Vulnerability details

Impact

When user's interaction type is ComputeOutputAmount, the unwrap fee applies twice and if the unwrap fee is 5% at first, it becomes 9.75% and users will receive 90.25% of amount instead of 95%.

Proof of Concept

When users provide some input token and get output token, they make an interaction. Here the interaction type is ComputeOutputAmount. And they call doInteraction function in the Ocean.sol.

File: src/ocean/Ocean.sol

210: function doInteraction(Interaction calldata interaction)
211:    external
212:    payable
213:    override
214:    returns (uint256 burnId, uint256 burnAmount, uint256 mintId, uint256 mintAmount)
215:    {
216:        emit OceanTransaction(msg.sender, 1);
217:        return _doInteraction(interaction, msg.sender);
218:    }

[210-218] And this calls _doInteraction function.

380:   function _doInteraction(
381:        Interaction calldata interaction,
382:        address userAddress
383:    ) 

412:  (inputToken, inputAmount, outputToken, outputAmount) = _executeInteraction(
413:                interaction, interactionType, externalContract, specifiedToken, 
      interaction.specifiedAmount, userAddress
414:  );

[380]

In the _executeInteration function they carry on calculation by interactionType.

597:   function _executeInteraction(
598:        Interaction memory interaction,
599:        InteractionType interactionType,
600:        address externalContract,
601:        uint256 specifiedToken,
602:        uint256 specifiedAmount,
603:        address userAddress
604:    )
605:        internal
606:        returns (uint256 inputToken, uint256 inputAmount, uint256 outputToken, uint256 outputAmount)
607:    {
608:        if (interactionType == InteractionType.ComputeOutputAmount) {
609:            inputToken = specifiedToken;
610:            inputAmount = specifiedAmount;
611:            outputToken = interaction.outputToken;
612:            outputAmount = _computeOutputAmount(
613:                externalContract, inputToken, outputToken, inputAmount, userAddress, interaction.metadata
614:            );
615:        } 

[597]

In the _computeOutPutAmount it calls IOceanPrimitive(primitive).computeOutputAmount function.

745: function _computeOutputAmount(
746:        address primitive,
747:        uint256 inputToken,
748:        uint256 outputToken,
749:        uint256 inputAmount,
750:        address userAddress,
751:        bytes32 metadata
752:    )
753:        internal
754:        returns (uint256 outputAmount)
755:    {
756:        // mint before making a external call to the primitive to integrate with external protocol primitive adapters
757:        _increaseBalanceOfPrimitive(primitive, inputToken, inputAmount);

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

762:        _decreaseBalanceOfPrimitive(primitive, outputToken, outputAmount);

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

[745-765]

IOceanPrimitive(primitive).computeOutputAmount function equals computeOutputAmount function in OceanAdapter.sol.

File: src/adapters/OceanAdapters.sol

55: function computeOutputAmount(
56:        uint256 inputToken,
57:        uint256 outputToken,
58:        uint256 inputAmount,
59:        address,
60:        bytes32 metadata
61:    )
62:        external
63:        override
64:        onlyOcean
65:        returns (uint256 outputAmount)
66:    {
67:        unwrapToken(inputToken, inputAmount);

        // handle the unwrap fee scenario
70:        uint256 unwrapFee = inputAmount / IOceanInteractions(ocean).unwrapFeeDivisor();
71:        uint256 unwrappedAmount = inputAmount - unwrapFee;

73:        outputAmount = primitiveOutputAmount(inputToken, outputToken, unwrappedAmount, metadata);

75:        wrapToken(outputToken, outputAmount);
76:    }

[55-76]

As we can see at line 67, we have to unwrap the token. Again another interaction for unwrapping the input token. And also there is the line for unwrap fee at 70 and the input amount decreases by the unwrap fee. But the problem is when calling unwrapToken, there is a fee collection.

File: src/adapters/Curve2PoolAdapter.sol.

121: function unwrapToken(uint256 tokenId, uint256 amount) internal override {
122:        address tokenAddress = underlying[tokenId];
123:
124:        Interaction memory interaction = Interaction({
125:            interactionTypeAndAddress: _fetchInteractionId(tokenAddress, uint256(InteractionType.UnwrapErc20)),
126:            inputToken: 0,
127:            outputToken: 0,
128:            specifiedAmount: amount,
129:            metadata: bytes32(0)
130:        });

132:        IOceanInteractions(ocean).doInteraction(interaction);
133:    }

[121-133]

To execute the above interaction, function _executeInteration in the Ocean.sol is called and this time the InteractionType is UnwrapErc20

File: src/ocean/Ocean.sol
628:   } else if (interactionType == InteractionType.UnwrapErc20) {
629:            inputToken = specifiedToken;
630:            inputAmount = specifiedAmount;
631:            outputToken = 0;
632:            outputAmount = 0;
633:            _erc20Unwrap(externalContract, inputAmount, userAddress, inputToken);

[628-633]

And finally in the _erc20Unwrap function there is a fee collection

864: function _erc20Unwrap(address tokenAddress, uint256 amount, address userAddress, uint256 inputToken) private {
865:        try IERC20Metadata(tokenAddress).decimals() returns (uint8 decimals) {
866:            uint256 feeCharged = _calculateUnwrapFee(amount);
867:            uint256 amountRemaining = amount - feeCharged;

869:            (uint256 transferAmount, uint256 truncated) =
870:                _convertDecimals(NORMALIZED_DECIMALS, decimals, amountRemaining);
871:            feeCharged += truncated;

873:            _grantFeeToOcean(inputToken, feeCharged);

875:            SafeERC20.safeTransfer(IERC20(tokenAddress), userAddress, transferAmount);
876:            emit Erc20Unwrap(tokenAddress, transferAmount, amount, feeCharged, userAddress, inputToken);
877:        } catch {
878:            revert NO_DECIMAL_METHOD();
879:        }
880:    }

[864-880]

As we can see at line 866 and 867, there is a fee collection and the amount is decreased by the fee.

But as we have seen earlier in IOceanPrimitive(primitive).computeOutputAmount, after unwrapToken function, there is fee collection again. And this means user has to pay the fee twice and will get the smaller amount than expected.

Tools Used

Manual review

Recommended Mitigation Steps

I think fee collection in _erc20Unwrap function is essential, so it is good to remove fee collection in the IOceanPrimitive(primitive).computeOutputAmount

File: src/adapters/OceanAdapters.sol

55: function computeOutputAmount(
56:        uint256 inputToken,
57:        uint256 outputToken,
58:        uint256 inputAmount,
59:        address,
60:        bytes32 metadata
61:    )
62:        external
63:        override
64:        onlyOcean
65:        returns (uint256 outputAmount)
66:    {
67:        unwrapToken(inputToken, inputAmount);

        // handle the unwrap fee scenario
-70:        uint256 unwrapFee = inputAmount / IOceanInteractions(ocean).unwrapFeeDivisor();
-71:        uint256 unwrappedAmount = inputAmount - unwrapFee;

-73:        outputAmount = primitiveOutputAmount(inputToken, outputToken, unwrappedAmount, metadata);

+70:        outputAmount = primitiveOutputAmount(inputToken, outputToken, inputAmount, metadata);

75:        wrapToken(outputToken, outputAmount);
76:    }

Assessed type

Other

[c4-bot-2]

Withdrawn by Weed0607