VIS-2 / taobank-04-24

0 stars 0 forks source link

Dutch auction poses disadvantages for bidders #9

Open DanailYordanov opened 4 months ago

DanailYordanov commented 4 months ago

Impact

Severity: High Likelihood: Medium

Context

AuctionManager::setLowestHealthFactor() AuctionManager::newAuction()

Description

During the initiation of a new Dutch auction for liquidation, the highest debt to be auctioned is set to be equal to the vault's stable coin debt:

uint256 _highestDebtToAuction = _debtToAuction;

and the lowest debt to be auctioned is equal to a percentage of the value of the collateral, calculated as follows:

uint256 _lowestDebtToAuction = (_totalCollateralValue * lowestHF) / DECIMAL_PRECISION;

When the health factor of a vault becomes < 1e18, the vault is liquidateable. However as the vault is overcollateralized (MCR > 100%) and there aren't significant price swings, the collateral remains worth more than the debt.

The lowestHF, representing a percentage of the collateral value, is initialized with a value of 1.05e18 (105%). The setter function setLowestHealthFactor() for this variable only checks whether the value is more than 0. Allowing lowestHF to be >= 1e18 (>= 100%), combined with an if-statement that reverses the price bounds (underflow prevention) and collateral already being worth more than the debt, causes the auction price to be more expensive than the collateral, resulting in bidders incurring losses instead of profit.

if (_highestDebtToAuction < _lowestDebtToAuction) {
    uint256 _debtToAuctionTmp = _lowestDebtToAuction;
    _lowestDebtToAuction = _highestDebtToAuction;
    _highestDebtToAuction = _debtToAuctionTmp; // @audit this is bigger than the actual debt
}

In scenarios where there are small price movements and a non-volatile market, and the health factor just dips under 1e18, the auction becomes unprofitable at the start and gradually becomes cheaper than the collateral value over time. However, in significant price swings and volatile markets, the lowest auction price remains higher than the collateral value, leading to bad debt.

Additionally, when lowestHF is == 1e18, the auction's price remains constant and does not change, rendering it ineffective as an auction mechanism.

Recommendation

Initialize lowestHF with a value < 1e18, for example:

uint256 public lowestHF = 0.95 ether; // 95%

Modify setLowestHealthFactor() as follows:

function setLowestHealthFactor(uint256 _lowestHF) external onlyOwner {
    require(_lowestHF > 0 && _lowestHF < 1e18, 'lowest-hf-is-0');
    lowestHF = _lowestHF;
}

PoC

To verify the issue, follow this guide and include this test in the codebase. This test demonstrates how even with a small price movement, the auction remains unprofitable for approximately 40 minutes out of a 2-hour-long auction, potentially resulting in bad debt if the price drops further during these 40 minutes.

DanailYordanov commented 4 months ago

Реално погледнато, мисля че релация между lowestHF и healthFactor(true), защото първото просто е някакъв процент от totalCollateralValue, а другото показва спрямо mlr-a и спрямо това колко си borrow-нал, дали не си надвишил този mlr следователно, това което съм написал в репорта, че имат някаква връзка не съм сигурен колко е вярвно, в тези калкулации би трябвало и да се включи mlr някакси.

0xMilenov commented 4 months ago

интересно братле браво

DanailYordanov commented 4 months ago

Душата си оставих да го дообясня и да оправя самия тест, че не ме кефеше много теста и как бях обяснил самия проблем. Утре ако можеш едитни ишуто в другото репо, че аз нямам право

0xMilenov commented 4 months ago

браво брат евала