Platform: Code4rena
Start Date: 27/11/2023
Pot Size: $36,500 USDC
Total HM: 0
Participants: 22
Period: 8 days
Judge: 0xA5DF
Id: 308
League: ETH
Rank: 8/22
Findings: 1
Award: $814.56
🌟 Selected for report: 0
🚀 Solo Findings: 0
814.5609 USDC - $814.56
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
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
.
outputAmount = _convertDecimals(decimals[outputToken], NORMALIZED_DECIMALS, rawOutputAmount);
} else { // Decimal shift right (remove precision) -> truncation uint256 shift = 10 ** (uint256(decimalsFrom - decimalsTo)); convertedAmount = amountToConvert / shift; }
outputAmount = _convertDecimals(decimals[outputToken], NORMALIZED_DECIMALS, rawOutputAmount);
function getTokenSupply(uint256 tokenId) external view override returns (uint256) { return 0; }
Manual Review and VSCode
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.
Other
#0 - c4-pre-sort
2023-12-10T03:26:45Z
raymondfam marked the issue as sufficient quality report
#1 - c4-pre-sort
2023-12-10T03:26:59Z
raymondfam marked the issue as duplicate of #252
#2 - c4-judge
2023-12-17T12:06:42Z
0xA5DF marked the issue as satisfactory
#3 - c4-judge
2023-12-17T12:11:21Z
0xA5DF changed the severity to 2 (Med Risk)
#4 - c4-judge
2023-12-18T08:48:54Z
0xA5DF changed the severity to QA (Quality Assurance)
#5 - c4-judge
2023-12-18T08:51:20Z
0xA5DF marked the issue as grade-c
#6 - 0xA5DF
2023-12-18T08:51:27Z
Moved to #277
#7 - c4-judge
2023-12-20T15:55:43Z
0xA5DF marked the issue as grade-b