code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/main/src/PrivatePool.sol#L484-L507 https://github.com/code-423n4/2023-04-caviar/blob/main/src/PrivatePool.sol#L127-L132

Vulnerability details

Impact

If a user is unaware that they are depositing assets into a private pool, they may essentially lose the assets they deposited.

Proof of Concept

https://docs.caviar.sh/technical-reference/custom-pools/smart-contract-api/privatepool#deposit

https://docs.caviar.sh/technical-reference/custom-pools/smart-contract-api/ethrouter#deposit

The misleading name and documentation on this function make it look like the function keeps you as a liquidity provider and earns you interest. After sending assets to the contract, they become the pool owner's property, and you no longer have access to them. This is what the documentation states about depositing directly in the private pool: "Deposit NFTs and base tokens (or ETH) into the pool to enable trading. Earn fees on each trade."

This is what the documentation says about depositing from the Eth router: "Executes a deposit to a private pool (transfers NFTs and ETH to the pool)."

Tools Used

Manual review

Recommended Mitigation Steps

Consider updating the documentation in a way that eliminates any potential misunderstandings.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #865

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)