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

7 stars 7 forks source link

Wrong implementation of `_computeInputAmount`. #248

Closed c4-bot-10 closed 9 months ago

c4-bot-10 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/ocean/Ocean.sol#L786-L806

Vulnerability details

Impact

Basically _computeInputAmount reverts because it tries to decrease token balance from the premitive in the beginning.

Proof of Concept

Usually Ocean's premitive contracts play a role of exchange, either swapping exact amount of tokens to another or swapping some amounts of tokens to exact amount of other token. Based on the goal, _computeInputAmount or _computeOutputAmount is called.

However in current implementation of _computeInputAmount, it decreases the output token balance of the premitive which will always revert, because premitive's token balances are always zero.

Tools Used

Manual Review

Recommended Mitigation Steps

For calculating input amount, interaction's metadata includes max input amount. Based on the information, the correct steps would look like follows:

  1. Increase the primitive's input token balance by the max input amount allowed by user.
  2. Execute exchange through the primitive, as a result, less than max input amount is consumed, exact output amount is generated.
  3. Decrease the primitive's input token balance by the delta between max input amount and actual required amount.
  4. Decrease the primitive's output token balance by output amount.

Assessed type

Context

c4-pre-sort commented 9 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 9 months ago

raymondfam marked the issue as duplicate of #53

c4-judge commented 9 months ago

0xA5DF changed the severity to QA (Quality Assurance)

c4-judge commented 9 months ago

0xA5DF marked the issue as grade-c

0xA5DF commented 9 months ago

Low quantity