Submitted by falconhoof, also found by pep7siup (1, 2), tnquanghuy0512, 0x6d6164616e, twicek, immeas, T1MOH, Krace, Baki, bitsurfer, and Beosin
There are two big issues in AccountingEngine.sol::auctionSurplus() when calculating values for the creating a Surplus Auction; specifically in Lines 213 - 217.
The maximum value for _params.surplusTransferPercentage, as checked by the function at line 199, is 1e18 so the check at line 213 will always return TRUE.
However, the check should return FALSE when _params.surplusTransferPercentage is 1e18 or 100% because in that case an auction shouldnt be created; rather the full surplusAmount should be transferred to extraSurplusReceiver in the next code block.
The use of ONE_HUNDRED_WAD causes the calculated figure to be 100 times greater than it should be.
Issue 1. For the first issue; when _params.surplusTransferPercentage is 100%, a ghost surplus Auction will be created and the entire surplusAmount amount will also be sent to the extraSurplusReceiver essentially double-counting a large amount of the surplus. This double accounting can destabilise the system and lead to underflows elsewhere.
Issue 2. Surplus auctions are created with massively inflated figures for _amountToSell. This has the potential to cause massive price imbalances and crash the protocol. There is potential here for a malicious actor to leverage the vulnerability to their advantage by creating lots of false surplus in system coins which they purchase cheaply.
The following combines both issues into one PoC to show the worst case scenario.
Given the following values and assuming the initial all checks pass in function before reaching Line 213 AccountingEngine.sol::auctionSurplus():
The following condition at Line 213 will always return TRUE:
So an auction will be created with _amountToSell 100 times higher than it should be:
Following this, a coin amount of surplusAmount is also sent to the extraSurplusReceiver calculated in Lines 224-231 which is actually the intended behaviour.
Foundry
Update the function on the two affected lines to use WAD instead of ONE_HUNDRED_WAD as:
RaymondFam (Lookout) commented:
pi0neerpat (OpenDollar) confirmed
MiloTruck (Judge) commented:
pi0neerpat (OpenDollar) commented
