code-423n4 / 2023-04-caviar-findings

9 stars 4 forks source link

what prevents private pool owner/user from depositing one or more NFTs into the pool using a nft parameter value different from the pool's nft variable value? I dont see any measures/checks to prevent this. #920

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/2cfa974fc146acb9dc53b7d410222483f4b15dd7/src/EthRouter.sol#L219

Vulnerability details

Impact

Not sure if there are any checks on the frontend GUI side for this? But if owner/user can successfully deposit NFTs from collection different than the pool's NFT collection, it not only could, but probably will cause problems, as it's clear to me that the private pools are designed(intentionally or unintentionally?) to only manage ONE NFT collection, and not multiple NFT collections. (By collection I obviously mean NFTs from the same mint, or same NFT contract.)

Proof of Concept

The nft parameter from the deposit function of the EthRouter contract is not checked anywhere between this function and the deposit function from the PrivatePool contract. It therefore seems entirely possible that the pool owner(if not users too, eventually), could deposit NFTs from different collection than what they currently have. It seems the protocol's current intention is to handle each private pool as if its NFTs from the same collection. All calculations too related to the customization settings a pool owner has access to.

Tools Used

VSC

Recommended Mitigation Steps

Simple check to ensure the nft parameter value is equal to the equivalent state variable from the private pool.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

GalloDaSballo commented 1 year ago

Would be something that the owner can do, and they can sweep the tokens back via execute not a vulnerability

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid