Caviar Private Pools - JGcarv's results

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/main/src/PrivatePool.sol#L459

Vulnerability details

Impact

Sellers that interact directly with the PrivatePool need to approve tokens beforehand, and due to the convenience, it's common practice to approveAll to avoid multiple transactions.

With that in mind, allowing the owner to execute arbitrary transactions, allows for taking tokens from previous sellers that haven't revoked the approval, effectively stealing them.

Proof of Concept

Tools Used

Manual review.

Recommended Mitigation Steps

Ideally, remove the execute function, as the potential flaws are greater than the possible benefit.

If that's not desired, then prevent calls to the pool's nft and baseToken

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/main/src/PrivatePool.sol#L459

Vulnerability details

Impact

Buyers that interact directly with the PrivatePool need to approve tokens beforehand, and due to the convenience, it's common practice to the maximum limit.

With that in mind, allowing the owner to execute arbitrary transactions, allows for taking erc20 tokens from previous buyers that haven't revoked the approval, effectively stealing them.

Proof of Concept

Tools Used

Manual review.

Recommended Mitigation Steps

Ideally, remove the execute function, as the potential flaws are greater than the possible benefit.

If that's not desired, then prevent calls to the pool's nft and baseToken

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/main/src/PrivatePool.sol#L737

Vulnerability details

Impact

The provided function changeFeeQuote returns different value for protocol fees than what should be charged, which affect all changes on private pools.

It currently takes the percentage of the feeAmount

protocolFeeAmount = feeAmount * Factory(factory).protocolFeeRate() / 10_000;

While other functions take the protocolFee from the total value of the purchase.

Recommended Mitigation Steps

Change to:

protocolFeeAmount = inputAmount * Factory(factory).protocolFeeRate() / 10_000;