User can unwrap tokens in small amounts to avoid paying fee thereby resulting in loss to the protocol

[c4-bot-8]

Lines of code

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

Vulnerability details

unwrapFeeDivisor is used to calculate the unwrap fee when unwrapping tokens by calling the associated functions like _erc20Unwrap(), _erc1155Unwrap() & _etherUnwrap(). However a user can unwrap the tokens by keeping the amount just 1 less than the divisor.

Proof of Concept

The unwrapFeeDivisor is initially set to type(uint256).max inside the constructor which can be set changed by the owner by calling the changeUnwrapFee().

File: Ocean.sol

    function changeUnwrapFee(uint256 nextUnwrapFeeDivisor) external override onlyOwner {
        /// @notice as the divisor gets smaller, the fee charged gets larger
        if (MIN_UNWRAP_FEE_DIVISOR > nextUnwrapFeeDivisor) revert();
        emit ChangeUnwrapFee(unwrapFeeDivisor, nextUnwrapFeeDivisor, msg.sender);
        unwrapFeeDivisor = nextUnwrapFeeDivisor;
    }

The _calculateUnwrapFee() simply divides the amount to be unwrapped by the divisor.

File: Ocean.sol

    function _calculateUnwrapFee(uint256 unwrapAmount) private view returns (uint256 feeCharged) {
        feeCharged = unwrapAmount / unwrapFeeDivisor;                                               
    }

Lets take the situation where an ERC1155 token is unwrapped.

File: Ocean.sol

    function _erc1155Unwrap(
        address tokenAddress,
        uint256 tokenId,
        uint256 amount,
        address userAddress,
        uint256 oceanId
    )
        private
    {
        if (tokenAddress == address(this)) revert NO_RECURSIVE_UNWRAPS();
        uint256 feeCharged = _calculateUnwrapFee(amount);       /// returns 0. [9999/10000]
        uint256 amountRemaining = amount - feeCharged;
        _grantFeeToOcean(oceanId, feeCharged);                  /// nothing is sent
        IERC1155(tokenAddress).safeTransferFrom(address(this), userAddress, tokenId, amountRemaining, "");
        emit Erc1155Unwrap(tokenAddress, tokenId, amount, feeCharged, userAddress, oceanId);
    }

• Owner sets unwrapFeeDivisor to 10000.
• User calls doInteraction() to unwrap its erc1155 token with interaction.specifiedAmount set to 9999.
• Inside _erc1155Unwrap(), _calculateUnwrapFee() is called with amount set to 9999.
• _calculateUnwrapFee() returns 0. [9999/10000 = 0]
• _grantFeeToOcean() is called with feeCharged set to 0.
• DAO's balance remains unchanged in this case.

In the situation where the user needs to unwrap a bigger amount, he can call doMultipleInteractions() with small amounts to avoid the fee.

Taking the above scenario, if the user wanted to unwrap tokens of amount 99990, he would call doMultipleInteractions() with interaction.specifiedAmount each having an amount 9999. This way he would have saved on the additional gas cost & avoid paying fee to the protocol.

Impact

User can avoid paying fee to the protocol by unwrapping in small amounts thereby resulting in a loss to the protocol. User can call the doMultipleInteractions reduce its gas costs compared to calling doInteraction()

Tools Used

Manual Review

Recommended Mitigation Steps

Add checks to make sure duplicate parameters aren't passed inside Interaction[]. Or any other suitable way to avoid this.

Assessed type

Error

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #31

[c4-judge]

0xA5DF marked the issue as unsatisfactory: Invalid

[Shubh0412]

The above submission is not a duplicate of #31

31 directly quotes the L-14 mentioned in the bot report.

However, the above submitted is a valid Medium because of the following reasons:

1. Users avoiding paying fee are directly responsible to loss of funds to the protocol.

2. During wrapping erc20 tokens, even the dust amount is sent as fees as can be seen below. That means the "protocol believes that transferring in small amounts would ultimately lead to transferrable units at a later stage". Had this functionality been absent or the protocol didn't care for the dust amount, the current issue could have been rejected but since the protocol itself takes sending dust amounts into consideration, the current issue is valid.

File: Ocean.sol

            // If the user is unwrapping a delta, the residual dust could be
            // written to the user's ledger balance. However, it costs the
            // same amount of gas to place the dust on the owner's balance,
            // and accumulation of dust may eventually result in
            // transferrable units again.
834:        _grantFeeToOcean(outputToken, dust);

Users who use this method to avoid paying the small fee, no matter how small the amount that would have ultimately resulted in a big amount results in a loss to the protocol.

3. During unwrapping the user has to calculate the amount + fees offchain before calling the respective unwrapping functions. That means the user can easily calculate the amount it needs to set to avoid paying fees to the ocean.

   File: Ocean.sol
   
    * @notice unwrap amounts may be subject to a fee that reduces the amount
    *  moved on the external token`s ledger. To unwrap an exact amount, the
    *  caller should compute off-chain what specifiedAmount results in the
    *  desired unwrap amount.

4. _doMultipleInteractions() makes it a lot easier to exploit this vulnerability because the user has to just call it once instead of repeatedly calling _doInteraction() because the former has no checks to ensure duplicate parameters & saves gas compared to the latter.

5. The vulnerability does not need elaborate planning or coding knowledge to be executed. Just basic maths. A user can simply tell others about this bug & gradually the protocol finds itself in a loss even though the no. of the users might be increasing but no fees is being sent.

[0xA5DF]

Got it, I think there was a similar issue that I marked as low/invalid since the extra gas would probably not be worth it for the attacker (even when doing multiple interactions in one tx). If you believe otherwise - please show me an example where it would be worth it.

[Shubh0412]

Suppose a user(whale) has deposited a lot of eth into the protocol & later decides to take it out.

While unwrapping, the user realizes that the fee he would be sending to the Ocean protocol is higher than the combined gas cost for calling the do multiple interaction functions with specific sets to avoid paying fee. Given that eth has a high monetary value compared to other tokens this results in loss to the protocol.

Given that the Ocean protocol is highly gas efficient compared to other protocols in the space, this vulnerability would be added bonus to the users given that the transactions which already cost less gas & now they do not have to pay any fee for using the protocol.

The Ocean protocol is highly dependent fees that is being sent to the owner (multisig wallet) during wrapping & unwrapping. In the case of tokens with high monetary value, the loss is higher.

In the near future with the Ocean protocol not showing monetary growth despite the increasing number of users who would keep exploiting this vulnerability, the protocol would end up in loss.

[0xA5DF]

the fee he would be sending to the Ocean protocol is higher than the combined gas cost for calling the do multiple interaction

How? Every interaction would save you just 1 wei in fees. Meaning, if we're talking about USDC that would be 1e-6 USD Meanwhile, even if we assume only 1000 gas units per interaction, with a gas price of 0.1 Gwei and ETH price of 1000$ that would still cost you 0.1*1e-9*1e3*1e3=1e-4 Meaning the gas would cost you 100 times the amount you saved with fees.

[Shubh0412]

No. Why would it save me just 1 wei?

Taking the PoC into consideration described,

• unwrapFeeDivisor set to 10,000 (1e4).
• Amount to be unwrapped is 1,000,000(1e6).
• doMultipleInteractions() is called with each set having an amount 9999.
• _calculateUnwrapFee() returns 0 for each set. [9999/10000 = 0]

Taking the gas fee for each set to be 0.0001 (1e-4) as described by you.

even if we assume only 1000 gas units per interaction, with a gas price of 0.1 Gwei and ETH price of 1000$ that would still cost you 0.1*1e-9*1e3*1e3=1e-4

• For amount of 1e6, doMultipleInteractions() will have 101 set [100 having amount 9999 & the last having amount of 100] (100*9999 + 1*100 = 1e6).
• So the gas price assuming each set costs 1e-4, the total gas cost for 101 sets will be = 0.0001*101 = 0.0101
• Since no fees is being sent the final cost to the user will be 0.0101 approx 1e-2.

However if the user had directly called doInteraction() without any manipulation to avoid fee,

• unwrapFeeDivisor set to 10,000 (1e4).
• Amount to be unwrapped is 1,000,000(1e6).
• _calculateUnwrapFee() returns 100 [1e6/1e4 = 1e2]
• _grantFeeToOcean() is called with feeCharged set to 100.
• Additional gas cost of 1e-4 for just calling doInteraction() once for this transaction.
• Total cost for the transaction (fee to ocean + gas) = 1e2 + 1e-4 = 100 + 0.0001 = 100.0001
• This way the cost turns out to be approx 1e2.

Exploiting the vulnerability, the total transaction cost to the user = 1e-2 WITHOUT exploiting the vulnerability, the total transaction cost to the user = 1e2

Thats a clear difference of 1000 times. The user saved 1000 times he would have payed by exploiting the bug & sending no fee to the protocol.

[0xA5DF]

Amount to be unwrapped is 1,000,000(1e6).

If you're talking about 1e6 wei then that's 1 USDC When you saved 100 wei you basically saved 1e-4 USD, which is way less than the extra gas cost

[Shubh0412]

@0xA5DF

Given the exact scenario in the above comments, one thing to note is that the protocol will be deployed on ARBITRUM where the token price is almost $1 - $1.1.

If we assume only 1000 gas units per interaction, with a gas price of 0.1 Gwei and ARBITRUM price of $1 that would cost 0.1*1e-9 * 1e3 * 1 = 1e-7

So the above example becomes

WITHOUT Sending Fee

• So the gas price assuming each set costs 1e-7, the total gas cost for 101 sets will be = 1e-7*101 = 0.0000101
• Since no fees is being sent the final cost to the user will be 0.0000101 approx 1e-5.

WITH Sending Fee

• Total cost for the transaction (fee to ocean[100 WEI] + gas) = 1e-4 + 1e-7 = 0.0001 + 0.0000001 = 0.0001001
• This way the cost turns out to be approx 1e-4 or 100 wei.

User deprived the protocol of 100 wei (1e-4 USD) in fee & saved himself 90 wei [1e-4 - 1e-5 = 0.00009 ]despite the additional gas cost.

Also according to the docs

The goal of Shell v3 is to make the Ocean compatible with external protocols through the use of adapter primitives.

If other external adapter primitives become aware of this issue, they can also use this vulnerability to exploit the Ocean protocol.

[0xA5DF]

ETH is the currency used for gas fees on Arbitrum and all Arbitrum transactions are powered by ETH.

Artbitrum FAQ