Manual NFT transferring can affect to the price

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L742-L746 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L230-L231 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L694-L706

Vulnerability details

Impact

In PrivatePool, the price is calculated based on virtualBaseTokenReserves and virtualNftReserves which are updated by buy/sell functions. There can be a difference between real asset and virtual reserves.

Proof of Concept

Example scenario: - A malicious user transfer 10 NFTs(as many as he can) to PrivatePool. - The malicious user call buy function to buy those 10 NFTs. (This doesn't revert because the smart contract can send those NFTs.) Let's say that virtualNftReserves is almost zero now. - A human wants to buy a NFT (which was originally in the pool). But it may reverts on L231 from uint underflow.

File: PrivatePool.sol
230:         virtualBaseTokenReserves += uint128(netInputAmount - feeAmount - protocolFeeAmount);
231:         virtualNftReserves -= uint128(weightSum);

In this case, no body is able to buy NFTs from the pool. Admin's manual update is required to solve it.

Tools Used

VSCode, Foundry

Recommended Mitigation Steps

We need to make a mapping variable like mapping(address => bool) listedIds which denotes the NFT ids only listed buy smart contract functions buy/sell/change/deposit/withdraw.

And don't allow any selling for unlisted ids.

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #96

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[c4-judge]

GalloDaSballo marked the issue as grade-c

[c4-judge]

GalloDaSballo marked the issue as grade-c