The function removeExcessBids may cause internal accounting inconsistencies
Description
The mapping bids keeps track of all current valid bids. Each bid includes the amount of reserve tokens the bidder wants to buy and the amount of coupon tokens the bidder wants to sell. The state variable totalSellReserveAmount records the total amount of reserve tokens in valid bids, and the state variable currentCouponAmount records the total amount of coupon tokens. These two state variables will be updated based on changes in bids.
function bid(uint256 buyReserveAmount, uint256 sellCouponAmount) external auctionActive returns(uint256) {
// [...]
currentCouponAmount += sellCouponAmount;
totalSellReserveAmount += buyReserveAmount;
if (bidCount > maxBids) {
if (lowestBidIndex == newBidIndex) {
revert BidAmountTooLow();
}
_removeBid(lowestBidIndex);
}
// Remove and refund out of range bids
removeExcessBids();
// [...]
}However, in the function removeExcessBids, if the proportion of the sell amount in a bid is removed, only sellCouponAmount and buyReserveAmount in bids[currentIndex] are updated, without updating the state variables currentCouponAmount and totalSellReserveAmount.
function removeExcessBids() internal {
// [...]
while (currentIndex != 0 && amountToRemove != 0) {
// Cache the current bid's data into local variables
Bid storage currentBid = bids[currentIndex];
uint256 sellCouponAmount = currentBid.sellCouponAmount;
uint256 prevIndex = currentBid.prevBidIndex;
if (amountToRemove >= sellCouponAmount) {
// [...]
} else {
// Calculate the proportion of sellAmount being removed
uint256 proportion = (amountToRemove * 1e18) / sellCouponAmount;
// Reduce the current bid's amounts
currentBid.sellCouponAmount = sellCouponAmount - amountToRemove;
currentBid.buyReserveAmount = currentBid.buyReserveAmount - ((currentBid.buyReserveAmount * proportion) / 1e18);
// Refund the proportional sellAmount
IERC20(buyCouponToken).safeTransfer(currentBid.bidder, amountToRemove);
amountToRemove = 0;
}
}
}Impact
If the auction succeeds, the pool will transfer totalSellReserveAmount amount of reserve tokens to the auction contract. Assuming a bid has x reserve tokens removed, all bids add up to only totalSellReserveAmount - x reserve tokens. The excess tokens cannot be claimed and will be locked in the Auction contract.
Recommendations
Consider updating the state variables currentCouponAmount and totalSellReserveAmount based on the changes in currentBid.sellCouponAmount and currentBid.buyReserveAmount.
Remediation
This issue has been acknowledged by Plaza Finance, and fixes were implemented in the following commits: