code-423n4 / 2022-10-inverse-findings

0 stars 0 forks source link

Returning users could get forced replenished #522

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/DBR.sol#L133-L138 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L559-L572

Vulnerability details

Impact

Currently, the balance of due DBR tokens for a user persists indefinitely with no way of decreasing it. A returning user who provides collateral before aquiring the DBR tokens to borrow will end up being forced replenished by automated market participants.

Proof of Concept

Assume a user borrows 10k DOLA for a year by providing enough collateral and buying 10k DBR. After exactly a year, they repay their debt, withdraw their collateral and sell their DBR on the market. Some time later they decide they want to borrow again and provide the necessary collateral as a first step (same thing as they did on their first borrow). This time around, they will get forced replenished, as they still had 10k of due DBR tokens that have accrued on their first borrow.

Note: The same issue would be present if they sell their DBR before withdrawing their collateral, because they thought everything would be settled after repaying their debt (because currently there is no way to repay due DBR tokens).

The Market.forceReplenish function checks if the user has a deficit and increases their debt as long as the minimum collateral is not exceeded:

function forceReplenish(address user, uint amount) public {
    uint deficit = dbr.deficitOf(user);
    require(deficit > 0, "No DBR deficit");
    require(deficit >= amount, "Amount > deficit");
    uint replenishmentCost = amount * dbr.replenishmentPriceBps() / 10000;
    ...
    debts[user] += replenishmentCost;
    uint collateralValue = getCollateralValueInternal(user);
    require(collateralValue >= debts[user], "Exceeded collateral value");

The DBR.deficitOf function returns the due tokens accrued in total by the user minus their current balance:

return dueTokensAccrued[user] + accrued - balances[user];

The value of dueTokensAccrued[user] in the scenario given above would be 10000*1e18 (10k DBR), which is also the value of the users deficit as both accrued and balances[user] are zero. This 10k DBR deficit can be forceReplenished to the max if the user provides enough collateral.

Tools Used

Manual Review

Recommended Mitigation Steps

  1. Consider automatically replenishing the users DBR at a lower rate than a force replenish would cost, when he deposits collateral while being in a deficit.
  2. Also consider providing a method to reduce the accrued DBR token amounts for user by burning DBR tokens. This would be especially useful for a user if the market rate for DBR has lowered since the time they bought it.
  3. Warn the user on the frontend side when this scenario is about to occur.
d3e4 commented 2 years ago

sell their DBR on the market

See my comment on #521 regarding the transfer of DBR-tokens.

c4-judge commented 2 years ago

0xean marked the issue as unsatisfactory: Invalid