code-423n4 / 2023-05-caviar-mitigation-contest-findings

0 stars 0 forks source link

Mitigation Confirmed for M-03 #33

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Mitigation of M-03: Issue mitigated

Issue

M-03: Flash loan fee is incorrect in Private Pool contract

Mitigation

https://github.com/outdoteth/caviar-private-pools/pull/6

Assessment of Mitigation

Issue mitigated

Comments

The mitigation updates the PrivatePool.flashFee function to the following code.

https://github.com/outdoteth/caviar-private-pools/pull/6/files#diff-6beea546a01e795e1365ca8a74b2bd952631f6cd228a5a869bfed82bf1f46a0fR753-R758

    function flashFee(address, uint256) public view returns (uint256) {
        // multiply the changeFee to get the fee per NFT (4 decimals of accuracy)
        uint256 exponent = baseToken == address(0) ? 18 - 4 : ERC20(baseToken).decimals() - 4;
        uint256 feePerNft = changeFee * 10 ** exponent;
        return feePerNft;
    }

After combining with other commits, the PrivatePool.flashFeeAndProtocolFee and PrivatePool.flashFee functions are currently as follows, where the PrivatePool.flashFeeAndProtocolFee function is called in the PrivatePool.flashLoan function.

https://github.com/outdoteth/caviar-private-pools/blob/main/src/PrivatePool.sol#L797-L812

    /// @notice Returns the fee and protocol fee required to flash swap a given NFT.
    /// @return feeAmount The fee amount.
    /// @return protocolFeeAmount The protocol fee amount.
    function flashFeeAndProtocolFee() public view returns (uint256 feeAmount, uint256 protocolFeeAmount) {
        // multiply the changeFee to get the fee per NFT (4 decimals of accuracy)
        uint256 exponent = baseToken == address(0) ? 18 - 4 : ERC20(baseToken).decimals() - 4;
        feeAmount = changeFee * 10 ** exponent;
        protocolFeeAmount = feeAmount * Factory(factory).protocolChangeFeeRate() / 10_000;
    }

    /// @notice Returns the fee required to flash swap a given NFT.
    /// @return feeAmount The fee amount.
    function flashFee(address, uint256) public view returns (uint256) {
        (uint256 feeAmount, uint256 protocolFeeAmount) = flashFeeAndProtocolFee();
        return feeAmount + protocolFeeAmount;
    }

In the PrivatePool.flashFeeAndProtocolFee function, uint256 exponent = baseToken == address(0) ? 18 - 4 : ERC20(baseToken).decimals() - 4 and feeAmount = changeFee * 10 ** exponent are executed, which scales the flash loan fee that excludes the protocol fee in the same way that the following PrivatePool.changeFeeQuote function scales the change fee per NFT. Hence, the PrivatePool.flashFeeAndProtocolFee and PrivatePool.changeFeeQuote functions are now consistent for scaling the respective fees, and the corresponding issue is mitigated.

https://github.com/outdoteth/caviar-private-pools/blob/main/src/PrivatePool.sol#L780-L787

    function changeFeeQuote(uint256 inputAmount) public view returns (uint256 feeAmount, uint256 protocolFeeAmount) {
        // multiply the changeFee to get the fee per NFT (4 decimals of accuracy)
        uint256 exponent = baseToken == address(0) ? 18 - 4 : ERC20(baseToken).decimals() - 4;
        uint256 feePerNft = changeFee * 10 ** exponent;

        feeAmount = inputAmount * feePerNft / 1e18;
        protocolFeeAmount = feeAmount * Factory(factory).protocolChangeFeeRate() / 10_000;
    }
c4-judge commented 1 year ago

GalloDaSballo marked the issue as satisfactory