Shell Protocol - Udsen's results

Shell Protocol is DeFi made simple. Enjoy powerful one-click transactions, unbeatably capital-efficient AMMs, and a modular developer experience.

General Information

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

Shell Protocol

Findings Distribution

Researcher Performance

Rank: 8/22

Findings: 1

Award: $814.56

QA:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: peanuts

Also found by: 0xmystery, EV_om, IllIllI, Udsen, bin2chen, lsaudit, oakcobalt

Labels

bug
downgraded by judge
grade-b
QA (Quality Assurance)
sufficient quality report
edited-by-warden
duplicate-252
Q-02

Awards

814.5609 USDC - $814.56

External Links

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

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

#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

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter