sherlock-audit / 2024-02-radicalxchange-judging

3 stars 1 forks source link

0xKartikgiri00 - `_checkBidAmount` function have a strict equality check, which cause revert when not really needed. #26

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

0xKartikgiri00

medium

_checkBidAmount function have a strict equality check, which cause revert when not really needed.

Summary

The _checkBidAmount function is checking if calculatedFeeAmount is strictly equal to the feeAmount. However, it is noticing that it's uncommon for the calculatedFeeAmount to be exactly equal to the provided fee amount cause of potential rounding errors or if the bidder has provided more feeAmount than calculatedFeeAmount.

Vulnerability Detail

The strict equality check (==) in the _checkBidAmount function can lead to the DOS when we actually do not need the revert cause what if the bidder has provided more feeAmount than calculatedFeeAmount. In this case _checkBidAmount function will revert even though the feeAmount is greater than the expected fees and it is really uncommon that bidder will provide the collateralAmount including the exact calculatedFeeAmount.

Impact

The placeBid function will revert unnecessarily due to minor difference in the calculatedFeeAmount and feeAmount. This will result in users being unable to place valid bids even though the fee amounts are reasonably close.

Code Snippet

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

Tool used

Manual Review

Recommendation

The recommended mitigation is to use ( <= ) instead of ( == ) operator cause it will allow the bidder to make the bid if the provided feeAmount is greater than the calculatedFeeAmount.

The recommended mitigation in code:

   function _checkBidAmount(
        uint256 bidAmount,//bid Amount
        uint256 feeAmount //Fee amount
    ) internal view returns (bool) {
        uint256 calculatedFeeAmount = _calculateFeeFromBid(bidAmount); 
+       return calculatedFeeAmount <= feeAmount;
-       return calculatedFeeAmount == feeAmount;
    }

Duplicate of #92