search

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

7 stars 7 forks source link

Limited functionality due to Primitive's balance being decreased before it is called #315

Closed c4-bot-1 closed 8 months ago

c4-bot-1 commented 8 months ago

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/ocean/Ocean.sol#L798

Vulnerability details

Impact

In the current implementation, the balance of a primitive is decreased (burned) before it is called. This requires the primitive to hold outputAmount of outputToken before a transaction, which severely restricts the functionality it can provide and has no clear advantage to the protocol.

This limits the flexibility and extensibility of the system, potentially preventing the integration of certain types of contracts or operations.

Proof of Concept

For instance, a pass-through Adapter similar to Curve2PoolAdapter and CurveTricryptoAdapter wrapping exact output swaps such as any of Uniswap's swap[Tokens,ETH]forExact[Tokens,ETH] would be impossible to implement under the current design.

Tools Used

Manual inspection

Recommended Mitigation Steps

Consider revising the order of operations to allow for more flexibility in the types of operations that primitives can perform. Specifically, the call to _decreaseBalanceOfPrimitive could be moved back to after the external call.

This would allow the implementation of more complex functionalities, such as pass-through Adapters. However, this change should be carefully evaluated for potential security risks and other implications.

Assessed type

Other

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

Readme:

Refactored the order in which a primitive's balances are updated. Previously, both mints and burns would occur after the primitive had performed its computation in computeOutputAmount or computeInputAmount. Now, the primitive's balances will be minted the input token or burned the output token before performing the computation step, and then will burn the output token or mint the input token based on the result.

c4-judge commented 8 months ago

0xA5DF marked the issue as unsatisfactory: Invalid

0xA5DF commented 8 months ago

As Raymond said, it seems like intended design