VIS-2 / taobank-04-24

0 stars 0 forks source link

Risk of auction duration modification in `AuctionManager` #31

Open DanailYordanov opened 4 months ago

DanailYordanov commented 4 months ago

Impact

Severity: High Likelihood: Medium

Context

AuctionManager::bid() AuctionManager::bidInfo() AuctionManager::setAuctionDuration()

Description

The AuctionManager::setAuctionDuration() function enables an admin to alter the duration of Dutch auctions. However, this flexibility poses risks to existing auctions. Modifying the auction duration mid-process could cause auctions to revert, execute at unfavorable prices, or conclude at higher prices, potentially discouraging users from participating in liquidations.

Recommendation

To mitigate these risks, implement caching of the auctionDuration at the time of auction creation in the auctionData struct. Utilize this cached variable within AuctionManager::bidInfo() and AuctionManager::bid(). This approach ensures that changes to the global auctionData won't impact existing auctions.

struct auctionData {
+   uint256 auctionDuration;
    uint256 originalDebt;
    uint256 lowestDebtToAuction;
    uint256 highestDebtToAuction;
    uint256 collateralsLength;
    address[] collateral;
    uint256[] collateralAmount;
    uint256 auctionStartTime;
    uint256 auctionEndTime;
    bool auctionEnded;
}

auctions.push(
    auctionData({
+       auctionDuration: auctionDuration,   
        originalDebt: _debtToAuction,
        lowestDebtToAuction: _lowestDebtToAuction,
        highestDebtToAuction: _highestDebtToAuction,
        collateralsLength: _collateralsLength,
        collateral: _collaterals,
        collateralAmount: _collateralAmounts,
        auctionStartTime: _auctionStartTime,
        auctionEndTime: _auctionEndTime,
        auctionEnded: false
    })
);

uint256 _debtToAuctionAtCurrentTime = _highestDebtToAuction -
    ((_highestDebtToAuction - _lowestDebtToAuction) *
        (block.timestamp - _auction.auctionStartTime)) /
-   auctionDuration; 
+  _auction.auctionDuration;

PoC

To verify the issue, follow this guide and include this test in the codebase. The test demonstrates how the protocol accrues bad debt by the inability to liquidate a vault when there is a huge change in the auction duration as well as by executing a liquidation at prices lower than the lowestDebtToAuction.

DanailYordanov commented 4 months ago

Moje bi e medium tva