Open Dollar - Saintcode_'s results

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/AccountingEngine.sol#L175-L193

Vulnerability details

The AccountingEngine.sol contract serves as the protocol's central component responsible for initializing both debt and surplus auctions. Debt auctions are consistently initiated with a predefined minimum bid referred to as debtAuctionBidSize. This is done to ensure that the protocol can only auction debt that is not currently undergoing an auction and is not locked within the debt queue, as articulated in the comment found on IAccountingEngine:248: "It can only auction debt that has been popped from the debt queue and is not already being auctioned". This necessity prompts the check on AccountingEngine:181:

if (_params.debtAuctionBidSize > _unqueuedUnauctionedDebt(_debtBalance))

This check verifies that there is a sufficient amount of bad debt available to auction.

The issue stems in the line 183, where _settleDebt is called, this aims to ensure that only bad debt is considered for the auction. However, if the remaining bad debt, after settlement, falls below the specified threshold (debtAuctionBidSize <= unqueuedUnauctionedDebt()), the auction still starts with an incorrect amount of bad debt coverage, diluting the protocol Token when it is not yet needed.

Impact

Non-existent debt gets auctioned when it is not necesary which dilutes the protocol token.

PoC

The attached Proof of Concept (PoC) demonstrates two test cases:

• In the initial test case, a two-call flow is implemented. The "settleDebt" function must be externally triggered before initiating an auction. This design ensures that a check for insufficient debt occurs prior to creating an auction. Consequently, as after calling settleDebt there is inadequate debt, the operation will revert.
• In the second case, the auctionDebt function is invoked directly. It is expected to behave in the same manner as in the first test case (because the funciton calls _settleDebt internally). However, in this second test case, the execution follows a different path. Rather than replicating the behavior of the initial test case by reverting not starting the auction, the execution succeeds, resulting in the creation of a debt auction even when there is no existing debt.

The following tests have been created using the protocol's test suite:

First test case (original two call flow):

  function test_missing_insuficient_debt_check_part2() public {
    accountingEngine.modifyParameters("surplusTransferPercentage", abi.encode(1));
    accountingEngine.modifyParameters("extraSurplusReceiver", abi.encode(1));

    safeEngine.createUnbackedDebt(address(0), address(accountingEngine), rad(100 ether));

    _popDebtFromQueue(100 ether);

    accountingEngine.settleDebt(100 ether);

    vm.expectRevert();
    uint id = accountingEngine.auctionDebt();

  }

Second test case (vulnerability):

  function test_missing_insuficient_debt_check_part2() public {
    accountingEngine.modifyParameters("surplusTransferPercentage", abi.encode(1));
    accountingEngine.modifyParameters("extraSurplusReceiver", abi.encode(1));

    safeEngine.createUnbackedDebt(address(0), address(accountingEngine), rad(100 ether));

    _popDebtFromQueue(100 ether);

    
    uint id = accountingEngine.auctionDebt();

  }

Both test cases should revert and not let a user create a debt Auction under insufficient debt circumstances, but as stated on the report the second test case succeeds and creates the Auction.

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate this risk, I suggest introducing the check (_params.debtAuctionBidSize > _unqueuedUnauctionedDebt(_debtBalance)) after calling _settleDebt to ensure there exists enough amount of bad in the contract after the settling.

Assessed type

Other