Al-Qa-qa - Bidders can pay less fees than required because of rounding down

[sherlock-admin2]

Al-Qa-qa

medium

Bidders can pay less fees than required because of rounding down

Summary

Because of Rounding down fees calculations, when calculating the required fee amount, the value can be less than required by the protocol.

Vulnerability Detail

When There is a new Bid, the protocol takes fees from the Bidder. But when calculating this fee, it is calculated by rounding down feeNumerator and feeDenominator.

EnglishPeriodicAuctionInternal.sol#L594-L603

    function _calculateFeeFromBid(uint256 bidAmount) internal view returns (uint256) {
        uint256 feeNumerator = IPeriodicPCOParamsReadable(address(this)).feeNumerator();
        uint256 feeDenominator = IPeriodicPCOParamsReadable(address(this)).feeDenominator();

        // @audit Rounding Down makes the value of fees decrease
        return (bidAmount * feeNumerator) / feeDenominator;
    }

Example

• Let's say the fee is 10% (feeNumerator = 1, feeDenominator = 10), with 4 Decimal Asset
• bidAmount = 99,999
• feeAmount = (bidAmount * feeNumerator) / feeDenominator = $(99,999 * 1) / 10 = 9,999$

• CollateralNeeded = bidAmount + feeAmount = $9,999 + 99,999 = 109,998$

• As we can see the collateralNeeded decreased by 2 units when the bidAmound is 99,999.
• If the bidAmound is 100,000 (greater than by just one unit, the feeAmount could be 10,000 and the CollateralNeeded would be 110,000.
• CollateralNeeeded decreased by 2 units instead of 1 unit because of rounding down.

Impact

Less fees collected by the protocol over time.

Code Snippet

https://github.com/sherlock-audit/2024-02-radicalxchange/blob/main/pco-art/contracts/auction/EnglishPeriodicAuctionInternal.sol#L602

Tool used

Manual Review

Recommendation

Always round in the favour of the protocol, not in the favour of the user, which is rounding Up instead of rounding Down.

You can use OpenZeppelin mulDiv function to multiply and then divide using Rounding Up.