Caviar Private Pools - Nyx's results

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L211-L289 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L301-L373

Vulnerability details

Impact

the royalty fees are calculated using the same sale price for each NFT, regardless of their individual weights.

uint256 salePrice = (netInputAmount - feeAmount - protocolFeeAmount) / tokenIds.length;

(uint256 royaltyFee, address recipient) = _getRoyalty(tokenIds[i], salePrice);

This can lead to incorrect royalty fee calculations if the NFTs have different weights, and more valuable NFT creators can lose funds.

Proof of Concept

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L235-L249

Tools Used

Manual Review

Recommended Mitigation Steps

uint256 salePriceWithoutWeight = netInputAmount - feeAmount - protocolFeeAmount;
uint256 royaltyFeeAmount = 0;
for (uint256 i = 0; i < tokenIds.length; i++) {
    // Calculate the sale price for each NFT based on its weight
    uint256 salePriceWithWeight = (totalCost * tokenWeights[i]) / weightSum;

    // Transfer the NFT to the caller
    ERC721(nft).safeTransferFrom(address(this), msg.sender, tokenIds[i]);

    if (payRoyalties) {
        // Get the royalty fee for the NFT
        (uint256 royaltyFee,) = _getRoyalty(tokenIds[i], salePriceWithWeight);

        // Add the royalty fee to the total royalty fee amount
        royaltyFeeAmount += royaltyFee;
    }
}

Consider calculating the salePrice for each NFT based on its weight.