Disruption Risk in Fee Recording through Malicious accrue Invocations

Summary

The accrue function pool.sol updates the lastUpdated timestamp every time it is called. This occurs regardless of whether any feeShares are minted. The function calculates accrued interest based on the time elapsed since lastUpdated and factors such as loaned assets and deposited assets. Calling accrue too frequently can lead to zero feeShares accruals due to the rounding down operations implemented in the contract.

Root Cause

https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/25a0c8aeaddec273c5318540059165696591ecfb/protocol-v2/src/Pool.sol#L401C5-L414C6


    function accrue(PoolData storage pool, uint256 id) internal {
        (uint256 interestAccrued, uint256 feeShares) = simulateAccrue(pool);

   @>>  if (feeShares != 0) _mint(feeRecipient, id, feeShares);

        // update pool state
        pool.totalDepositShares += feeShares;
        pool.totalBorrowAssets += interestAccrued;
        pool.totalDepositAssets += interestAccrued;

        // store a timestamp for this accrue() call
        // used to compute the pending interest next time accrue() is called
   @>>  pool.lastUpdated = uint128(block.timestamp);
    }

https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/25a0c8aeaddec273c5318540059165696591ecfb/protocol-v2/src/Pool.sol#L380C5-L398C6


    function simulateAccrue(PoolData storage pool) internal view returns (uint256, uint256) {
        uint256 interestAccrued = IRateModel(pool.rateModel).getInterestAccrued(
            pool.lastUpdated, pool.totalBorrowAssets, pool.totalDepositAssets
        );

        uint256 interestFee = pool.interestFee;
        if (interestFee == 0) return (interestAccrued, 0);
        // [ROUND] floor fees in favor of pool lenders
        uint256 feeAssets = interestAccrued.mulDiv(pool.interestFee, 1e18);
        // [ROUND] round down in favor of pool lenders
        uint256 feeShares = _convertToShares(
            feeAssets,
            pool.totalDepositAssets + interestAccrued - feeAssets,
            pool.totalDepositShares,
    @>>     Math.Rounding.Down
        );

        return (interestAccrued, feeShares);
    }

Internal pre-conditions

Time Interval Sensitivity: The accrue function is sensitive to the time intervals at which it is invoked because it updates the lastUpdated timestamp and recalculates fees based on the elapsed time since the last update.

Fee Calculation Mechanism: The contract uses a fee calculation mechanism (_convertToShares) that rounds down the accrued interest to determine the number of fee shares. If the interest accrued over a short interval is minimal, this rounding can result in zero fee shares, even though an update still occurs.

External pre-conditions

Accessible accrue Function: The accrue function must be publicly callable or accessible enough that an external actor can invoke it repeatedly in a short span. This includes any permissions or exposure within the contract that allows for frequent triggering of this function without substantial economic activity.

Attack Path

Preparation: The attacker identifies the Pool contract with the vulnerable accrue function and understands its fee calculation and update mechanism.

Frequent Invocation: The attacker begins to repeatedly invoke the accrue function at intervals short enough that the interest calculated results in zero fee shares due to rounding down.

Stamp Update Without Economic Change: Each call updates the lastUpdated timestamp without corresponding economic changes (i.e., no actual fee shares are created), effectively resetting the interval for interest calculations.

Impact

Misrepresentation of Fee Distribution: When the accrue function is called repeatedly within short intervals, the actual accrual of interest may not be properly captured due to the rounding down to zero fee shares. This misalignment occurs even though, cumulatively, these frequent accruals should have resulted in some fee shares being recorded. Over time, this leads to a significant discrepancy in the intended versus actual distribution of fees among pool participants.

PoC

No response

Mitigation

Only update the pool state if at least some feeShares are available.


function accrue(PoolData storage pool, uint256 id) internal {
        (uint256 interestAccrued, uint256 feeShares) = simulateAccrue(pool);

      if (feeShares != 0) {

        _mint(feeRecipient, id, feeShares);

        // update pool state
        pool.totalDepositShares += feeShares;
        pool.totalBorrowAssets += interestAccrued;
        pool.totalDepositAssets += interestAccrued;

        // store a timestamp for this accrue() call
        // used to compute the pending interest next time accrue() is called
        pool.lastUpdated = uint128(block.timestamp);
      }
    }

sherlock-admin4 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

z3s commented:

Low/Info; getInterestAccrued round up the interestAccrued so it's only zero share when block.timestamp == lastUpdated, so the attacker cannot always keep it at zero

sherlock-admin2 commented 2 weeks ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/sentimentxyz/protocol-v2/pull/325

sherlock-audit / 2024-08-sentiment-v2-judging

Galturok - Disruption Risk in Fee Recording through Malicious accrue Invocations #16