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

2 stars 0 forks source link

An expired parameter is required because there may be slippage in the calculation. #415

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/RewardsManager.sol#L561-L598

Vulnerability details

Impact

Due to changes in interest rates, failure to process transactions in a timely manner may result in missing out on ideal rewards.

Proof of Concept

The calculation of the _clainRewards function involves interest rates, which are variable. If _updateBucketExchangeRates is not executed in a timely manner, the bucketRate read in the next step _calculateNextEpochRewards may not be what we want, which will affect the results of rewardsEarned.

Tools Used

vscode + foundry

Recommended Mitigation Steps

function _claimRewards(
        StakeInfo storage stakeInfo_,
        uint256 tokenId_,
        uint256 epochToClaim_,
        bool validateEpoch_,
        address ajnaPool_,
        //@audit
        uint256 expired
    ) internal {
        //@audit
        require(block.timestamp < expired, "expired");
        // revert if higher epoch to claim than current burn epoch
        if (validateEpoch_ && epochToClaim_ > IPool(ajnaPool_).currentBurnEpoch()) revert EpochNotAvailable();

        // update bucket exchange rates and claim associated rewards
        uint256 rewardsEarned = _updateBucketExchangeRates(
            ajnaPool_,
            positionManager.getPositionIndexes(tokenId_)
        );

        rewardsEarned += _calculateAndClaimRewards(tokenId_, epochToClaim_);

        uint256[] memory burnEpochsClaimed = _getBurnEpochsClaimed(
            stakeInfo_.lastClaimedEpoch,
            epochToClaim_
        );

        emit ClaimRewards(
            msg.sender,
            ajnaPool_,
            tokenId_,
            burnEpochsClaimed,
            rewardsEarned
        );

        // update last interaction burn event
        stakeInfo_.lastClaimedEpoch = uint96(epochToClaim_);

        // transfer rewards to sender
        _transferAjnaRewards(rewardsEarned);
    }

Control the execution of the function by using the condition "block.timestamp < expired".

Assessed type

MEV

c4-judge commented 1 year ago

Picodes changed the severity to 2 (Med Risk)

c4-sponsor commented 1 year ago

MikeHathaway marked the issue as sponsor acknowledged

Picodes commented 1 year ago

The impact described isn't sufficient to justify Medium severity in my opinion.

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Insufficient proof