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

0 stars 0 forks source link

DBR `totalSupply` function doesn't factor pending accrued debt #534

Closed code423n4 closed 1 year ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L109

Vulnerability details

Impact

The DBR contract token balances are defined by the underlying balances minus the accrued debt generated by loans, effectively giving a semantic of "burned" tokens over time to cover the interests from loans taken by the users.

The function balanceOf (and similarly deficitOf and signedBalanceOf) track this by taking the internal balance (balances mapping) and subtracting the debt: the one already accrued (dueTokensAccrued mapping) and the pending debt since the last update (block.timestamp - lastUpdated[user]) * debt / 365 days).

However, the totalSupply() function fails to factor the pending debt, it takes the internal _totalSupply and subtracts only the totalDueTokensAccrued which is the total already accrued user debt.

This means that the sum of user balances (∑ balanceOf(user) for all users) isn't equal (in all cases) to totalSupply() (given the same context and block.timestamp)

Proof of Concept

This small test reproduces the issue. Here a user takes a loan and, after some time, if accrueDueTokens(user) isn't called for that user, balanceOf(user) will correctly report the value, but totalSupply() won't match it.

contract AuditTest is FiRMTest {
    function setUp() public {
        initialize(replenishmentPriceBps, collateralFactorBps, replenishmentIncentiveBps, liquidationBonusBps, callOnDepositCallback);

        vm.startPrank(chair);
        fed.expansion(IMarket(address(market)), 1_000_000e18);
        vm.stopPrank();
    }

    function testDBRBalanceVsTotalSupply() public {
        uint256 initialUserDbrBalance = wethTestAmount * ethFeed.latestAnswer() / 1 ether;

        gibWeth(user, wethTestAmount);
        gibDBR(user, initialUserDbrBalance);

        // No debts, total supply is owned by user
        console.log("DBR totalDueTokensAccrued: ", dbr.totalDueTokensAccrued());
        console.log("DBR totalSupply: ", dbr.totalSupply());
        console.log("DBR User balance ", dbr.balanceOf(user));

        uint256 initialTotalSupply = dbr.totalSupply();

        assertEq(dbr.totalSupply(), dbr.balanceOf(user));

        vm.startPrank(user);

        deposit(wethTestAmount);
        uint borrowAmount = wethTestAmount * ethFeed.latestAnswer() * collateralFactorBps / 1e18 / 10_000;
        market.borrow(borrowAmount / 2);

        // no call to accrueDueTokens(user) during a period of time
        vm.warp(block.timestamp + 30 days);

        console.log("----------AFTER----------");

        // Total supply is still initial value
        console.log("DBR totalSupply: ", dbr.totalSupply());
        assertEq(dbr.totalSupply(), initialTotalSupply);

        // User's balance has the pending accrued debt
        uint256 pendingDeficit = (block.timestamp - dbr.lastUpdated(user)) * dbr.debts(user) / 365 days;
        assertEq(dbr.balanceOf(user), initialUserDbrBalance - pendingDeficit);

        // These should match!
        assertEq(dbr.totalSupply(), dbr.balanceOf(user));
    }
}

Recommended Mitigation Steps

The totalSupply() should factor pending debt as balanceOf(user) does, so that it correctly reports its value.

c4-judge commented 2 years ago

0xean marked the issue as duplicate

c4-judge commented 1 year ago

0xean marked the issue as satisfactory

c4-judge commented 1 year ago

0xean marked the issue as not a duplicate

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2022-10-inverse-findings/issues/536