Eliminate dust when wrap ERC-20 token by retaining erc20 balance in their respective decimal places

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/ocean/Ocean.sol#L820-L842
https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/ocean/Ocean.sol#L622-L628
https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/ocean/Ocean.sol#L1068-L1109

Vulnerability details

Impact

The creation of token dust through decimal conversion poses two key issues:

Ownership - Dust gets sent to the contract owner rather than users, despite being their rightful property.

Restricted movement - Dust accumulates and cannot be withdrawn until reaching 1 WEI minimum, inconveniencing the owner of dust.

Proof of Concept

Dust gets created when:

Users wrap ERC20 tokens into Ocean via doInteraction() or doMultipleInteractions() with the WrapErc20 interaction type.

Here the token amount gets converted, with leftovers assigned as dust sent to the owner, as seen in this snippet:

function _erc20Wrap(address tokenAddress, uint256 amount, address userAddress, uint256 outputToken) private {
        try IERC20Metadata(tokenAddress).decimals() returns (uint8 decimals) {
            /// @dev the amount passed as an argument to the external token
            uint256 transferAmount;
            /// @dev the leftover amount accumulated by the Ocean.
            uint256 dust;

            (transferAmount, dust) = _determineTransferAmount(amount, decimals);

            // 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.
            _grantFeeToOcean(outputToken, dust);

            SafeERC20.safeTransferFrom(IERC20(tokenAddress), userAddress, address(this), transferAmount);

            emit Erc20Wrap(tokenAddress, transferAmount, amount, dust, userAddress, outputToken);
        } catch {
            revert NO_DECIMAL_METHOD();
        }
    }

Tools Used

Manual

Recommended Mitigation Steps

Dust needs to be removed. External tokens should be stored in the Ocean ERC-1155 contract at their original precision, avoiding any decimal conversion or normalization.

Suggest to change

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

-            (uint256 transferAmount, uint256 truncated) =
-                _convertDecimals(NORMALIZED_DECIMALS, decimals, amountRemaining);
-            feeCharged += truncated;

-            _grantFeeToOcean(inputToken, feeCharged);

-            SafeERC20.safeTransfer(IERC20(tokenAddress), userAddress, transferAmount);
+            SafeERC20.safeTransfer(IERC20(tokenAddress), userAddress, amountRemaining);

            emit Erc20Unwrap(tokenAddress, transferAmount, amount, feeCharged, userAddress, inputToken);
        } catch {
            revert NO_DECIMAL_METHOD();
        }
    }
``








## Assessed type

Other

[raymondfam]

Intended per the comment:

            // 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.

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-judge]

0xA5DF marked the issue as unsatisfactory:
Invalid