code-423n4 / 2023-05-ajna-findings

2 stars 0 forks source link

Arithmetic Underflow Vulnerability in _getPoolAccumulators Function #406

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/RewardsManager.sol#L636-L661

Vulnerability details

Impact

In the _getPoolAccumulators function, the values of totalBurnedLatest and totalInterestLatest are not checked for underflow before subtracting totalBurnedAtBlock and totalInterestAtBlock, respectively, this could result in an arithmetic underflow if totalBurnedAtBlock or totalInterestAtBlock is greater than totalBurnedLatest or totalInterestLatest, respectively.

    function _getPoolAccumulators(
        address pool_,
        uint256 currentBurnEventEpoch_,
        uint256 lastBurnEventEpoch_
    ) internal view returns (uint256, uint256, uint256) {
        (
            uint256 currentBurnTime,
            uint256 totalInterestLatest,
            uint256 totalBurnedLatest
        ) = IPool(pool_).burnInfo(currentBurnEventEpoch_);

        (
            ,
            uint256 totalInterestAtBlock,
            uint256 totalBurnedAtBlock
        ) = IPool(pool_).burnInfo(lastBurnEventEpoch_);

        uint256 totalBurned   = totalBurnedLatest   != 0 ? totalBurnedLatest   - totalBurnedAtBlock   : totalBurnedAtBlock;
        uint256 totalInterest = totalInterestLatest != 0 ? totalInterestLatest - totalInterestAtBlock : totalInterestAtBlock;

        return (
            currentBurnTime,
            totalBurned,
            totalInterest
        );
    }

In this case, an underflow could occur if totalBurnedAtBlock or totalInterestAtBlock is greater than totalBurnedLatest or totalInterestLatest, respectively, which can happen if the contract's state is manipulated or if the contract's logic contains a bug.

If an attacker is able to trigger an underflow, they could potentially steal tokens or cause the contract to malfunction. For example, an attacker could manipulate the contract's state to set totalBurnedAtBlock or totalInterestAtBlock to a very large value, causing an underflow when the function attempts to subtract them from totalBurnedLatest or totalInterestLatest, respectively. The resulting negative value could then be used by the attacker to steal tokens or perform other malicious actions.

Proof of Concept

The _getPoolAccumulators function, the values of totalBurnedLatest and totalInterestLatest are not checked for underflow before subtracting totalBurnedAtBlock and totalInterestAtBlock, respectively. This could result in an arithmetic underflow if either totalBurnedAtBlock or totalInterestAtBlock is greater than its corresponding value in totalBurnedLatest or totalInterestLatest, respectively.

For instance, let's consider the following scenario:

Reference code of the _getPoolAccumulators function: https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/RewardsManager.sol#L636-L661

Tools Used

vscode

Recommended Mitigation Steps

Add a check to ensure that totalBurnedLatest is greater than or equal to totalBurnedAtBlock and that totalInterestLatest is greater than or equal to totalInterestAtBlock before subtracting the latter from the former.

Assessed type

Under/Overflow

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid