Caviar Private Pools - tallo's results

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/main/src/EthRouter.sol#L118-L123 https://github.com/code-423n4/2023-04-caviar/blob/main/src/EthRouter.sol#L185-L190 https://github.com/code-423n4/2023-04-caviar/blob/main/src/PrivatePool.sol#L277 https://github.com/code-423n4/2023-04-caviar/blob/main/src/EthRouter.sol#L311

Vulnerability details

Inside EthRouter.sol, the buy and sell functions contain logic to transfer royalty fees to the respected NFT creator. This royaltyFee and royaltyRecipient are calculated by calling ERC2981's royaltyInfo function.

(recipient, royaltyFee) = IERC2981(lookupAddress).royaltyInfo(tokenId, salePrice);

royaltyinfo can return address(0) as the royalty recipient address which will lead to the sent ethereum being lost

Impact

Sent royalty fees will be burned

Proof of Concept

                if (payRoyalties) {
                    uint256 salePrice = inputAmount / buys[i].tokenIds.length;
                    for (uint256 j = 0; j < buys[i].tokenIds.length; j++) {
                        // get the royalty fee and recipient
                        (uint256 royaltyFee, address royaltyRecipient) =
                            getRoyalty(buys[i].nft, buys[i].tokenIds[j], salePrice);

                        //@audit here there should be a check that the royaltyRecipient isnt address(0)
                        if (royaltyFee > 0) {
                            // transfer the royalty fee to the royalty recipient
                            royaltyRecipient.safeTransferETH(royaltyFee);
                        }
                    }

Tools Used

Recommended Mitigation Steps

Check that the royalty recipient is not address(0) as is done correctly in PrivatePool.sol

 if (royaltyFee > 0 && recipient != address(0))