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

7 stars 7 forks source link

Upgraded Q -> 2 from #336 [1702651073475] #338

Closed c4-judge closed 8 months ago

c4-judge commented 8 months ago

Judge has assessed an item in Issue #336 as 2 risk. The relevant finding follows:

Potential Overcharge on Unwrapping Fee

0xA5DF commented 8 months ago

Full relevant part from the report:

[L-01] Potential Overcharge on Unwrapping Fee

Context

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

Description

In the computeOutputAmount function of OceanAdapter.sol, the unwrappedAmount is calculated by subtracting the unwrapFee from the inputAmount. This unwrappedAmount is then passed as a parameter to the primitiveOutputAmount function. However, the unwrap fee may be higher than the unwrappedAmount due to truncation, in which case the value passed to the primitiveOutputAmount function is higher than the actual available amount.

The code in question:

uint256 unwrapFee = inputAmount / IOceanInteractions(ocean).unwrapFeeDivisor(); uint256 unwrappedAmount = inputAmount - unwrapFee; outputAmount = primitiveOutputAmount(inputToken, outputToken, unwrappedAmount, metadata);

The code implicitly assumes that Adapters need to truncate the unwrappedAmount to the token's decimals.

Recommendation

Instead of making the assumption about the truncation, the unwrapped amount should be inferred from the balance change and passed to primitiveOutputAmount, ideally already in the token's precision. This would ensure that the correct amount is used in the calculations and prevent potential overcharging.

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

This auto-generated issue was withdrawn by 0xA5DF

c4-judge commented 8 months ago

0xA5DF marked the issue as grade-c

0xA5DF commented 8 months ago

Moved to #336