search

code-423n4 / 2023-04-caviar-findings

9 stars 4 forks source link

INACCURATE ARITHMETIC OPERATION IN CHANGEFEEQUOTE #942

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

There is an incorrect use of variable in the arithmetic calculation pertaining to assigning protocolFeeAmount in changeFeeQuote(). Although it does not currently affect the protocol, it will when the Factory owner decides to start collecting protocol fees.

Proof of Concept

feeAmount is first correctly computed after adjusting changeFee to the correct exponent. However, instead of using inputAmount, the function logic uses the diminished feeAmount to multiply with Factory(factory).protocolFeeRate(). This will make protocolFeeAmount very much smaller than expected possibly as good as nothing, defeating the purpose of introducing it to the system.

PrivatePool.sol#L731-L738

    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).protocolFeeRate() / 10_000;
    }

Recommended Mitigation Steps

It is recommended replacing feeAmount with inputAmount like it has been done so when calculating for feeAmount.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #463

c4-judge commented 1 year ago

GalloDaSballo marked the issue as satisfactory