c4-bot-6 commented 8 months ago

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/Curve2PoolAdapter.sol#L173 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/OceanAdapter.sol#L154-L158 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/CurveTricryptoAdapter.sol#L225 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/OceanAdapter.sol#L124-L126

Vulnerability details

Impact

The Curve2PoolAdapter.primitiveOutputAmount function performs the exchange of the inputAmount of the inputToken to the respective amount of the outputToken via the curve liquidity pool based on the action (eg: swap, deposit, withdraw).

Once the rawOutputAmount is received via the curve pool the rawOutputAmount value is converted into the NORMALIZED_DECIMALS value as shown below:

    outputAmount = _convertDecimals(decimals[outputToken], NORMALIZED_DECIMALS, rawOutputAmount);

But the issue here is that if the decimals[outputToken] > NORMALIZED_DECIMALS then the rawOutputAmount will be divided by the uint256(decimals[outputToken] - NORMALIZED_DECIMALS) value thus introducing rounding error (truncation of value) in the outputAmount due to division.

The Ocean._erc20Wrap function is called after the curve liquidity pool transaction to wrap the output token amount. During the execution of this function the Ocean._determineTransferAmount function is called which converts the decimals of the output token amount to its unscaled decimal value from NORMALIZED_DECIMALS. In this calculation if the NORMALIZED_DECIMALS > decimals then the amount is divided by the NORMALIZED_DECIMALS - decimals value thus introducing rounding error. But this truncated value is used to calculate the dust and transferred the dust amount as fee to the ocean protocol.

Furthermore the OceanAdapter.getTokenSupply function returns 0 as shown below:

function getTokenSupply(uint256 tokenId) external view override returns (uint256) {
    return 0; //@audit-info - returns zero because the primitive does not have any tokens since everything has been swapped or transferred
}

This indicates that the OceanAdapter contract should not hold any erc20 tokens. Since the Curve2PoolAdapter contract and CurveTricryptoAdapter contracts inherit from the OceanAdapter contract, this means that there should not be any leftover tokens in either of the Curve2PoolAdapter contract or CurveTricryptoAdapter contract. But due to the above mentioned truncation there is left over tokens of the output token stuck in the Curve2PoolAdapter contract or CurveTricryptoAdapter contract.

Similar issue is found in the CurveTricryptoAdapter.primitiveOutputAmount function as well. The truncation which occurs during the decimal conversion of the output token amount in this function is also ignored thus leading to loss of funds to the protocol.

But the same truncation value of the output token, is ignored during the decimal conversion in the Curve2PoolAdapter.primitiveOutputAmount function and it is left in the Curve2PoolAdapter contract (as explained earlier) since the dust value is not calculated and charged as fee to the Ocean protocol. This truncated value is also not accounted for in the Ocean._erc20Wrap since it will result in NORMALIZED_DECIMALS < decimals which won't introduce any truncation. The truncation happened inside the Curve2PoolAdapter.primitiveOutputAmount function which was not accounted for. Hence this is loss of funds to the ocean protocol.

Proof of Concept

        outputAmount = _convertDecimals(decimals[outputToken], NORMALIZED_DECIMALS, rawOutputAmount);

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

        } else {
            // Decimal shift right (remove precision) -> truncation
            uint256 shift = 10 ** (uint256(decimalsFrom - decimalsTo));
            convertedAmount = amountToConvert / shift;
        }

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

        outputAmount = _convertDecimals(decimals[outputToken], NORMALIZED_DECIMALS, rawOutputAmount);

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

    function getTokenSupply(uint256 tokenId) external view override returns (uint256) {
        return 0;
    }

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

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

But this truncated value is lost funds to the protocol and this also should be charged as fee to the protocol else it will be lost forever. This truncated value is not considered in the dust value of the Ocean._determineTransferAmount value calculation. Hence it is recommended to update the Curve2PoolAdapter.primitiveOutputAmount function implementation and calculate the dust value when the truncation happens during decimal conversion and charge that dust amount as fee to the Ocean protocol. The truncation will happen when the decimals[outputToken] > NORMALIZED_DECIMALS as explained earlier.

Assessed type

Other

c4-pre-sort commented 8 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #252