code-423n4 / 2023-11-shellprotocol-findings

7 stars 7 forks source link

Truncated value is not returned #318

Closed c4-bot-4 closed 8 months ago

c4-bot-4 commented 8 months ago

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/OceanAdapter.sol#L138-L159 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/Curve2PoolAdapter.sol#L142-L184 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/CurveTricryptoAdapter.sol#L178-L236

Vulnerability details

Impact

Detailed description of the impact of this finding.

In _convertDecimal() function of OceanAdapter.sol truncated amount is not returned but it should be returned as mentioned in the comment "returning the truncated amount if a truncation occurs."

Also in primitiveOutputAmount() function of Curve2PoolAdapter.sol and CurveTricryptoAdapter.sol contracts If decimal[inputToken] < NORMALIZED_DECIMALS then there will be truncated amount > 0 which is not taken into consideration.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/OceanAdapter.sol#L138-L159 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/Curve2PoolAdapter.sol#L142-L184 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/CurveTricryptoAdapter.sol#L178-L236

Tools Used

Manual

Recommended Mitigation Steps

truncatedAmount = amountToConvert % shift; should be added in OceanAdapter.sol when decimalFrom > decimalTo so that truncated amount can be handled properly when in other contracts _convertDecimal() function is called.

Assessed type

Context

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as primary issue

raymondfam commented 8 months ago

It only applies to Ocean.sol. Insufficient proof to be deduped under #252.

c4-judge commented 8 months ago

0xA5DF marked the issue as duplicate of #252

c4-judge commented 8 months ago

0xA5DF marked the issue as satisfactory

c4-judge commented 8 months ago

0xA5DF changed the severity to QA (Quality Assurance)

c4-judge commented 8 months ago

0xA5DF marked the issue as grade-c

0xA5DF commented 8 months ago

Low quantity