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

2 stars 0 forks source link

Uninitialized Variable Vulnerability in _updateBucketExchangeRates function #408

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#L671-L735 https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/RewardsManager.sol#L680-L729

Vulnerability details

Impact

In the function _updateBucketExchangeRates, the variable updatedRewards_ is not initialized before it is used in the function, this means that its value could be arbitrary and potentially manipulated by an attacker. This is a problem because the updatedRewards_ variable is used to track the amount of rewards earned by users who update the bucket exchange rate, and it's important that this value is accurate and not subject to manipulation.

If an attacker is able to manipulate the updatedRewards_ variable, they could potentially earn more rewards than they are entitled to, or even cause the contract to behave in unexpected ways that could lead to the theft of funds.

if the if (block.timestamp <= curBurnTime + UPDATE_PERIOD) condition evaluates to false, then the function returns the uninitialized value of updatedRewards_, which could cause unexpected behavior or vulnerabilities in the contract. The code block is as follows: #L680-L729

// update exchange rates and calculate rewards if tokens were burned and within allowed time period
for (uint256 i = 0; i < indexes_.length; ) {

    // calculate rewards earned for updating bucket exchange rate
    updatedRewards_ += _updateBucketExchangeRateAndCalculateRewards(
        pool_,
        indexes_[i],
        curBurnEpoch,
        totalBurned,
        totalInterestEarned
    );

    // iterations are bounded by array length (which is itself bounded), preventing overflow / underflow
    unchecked { ++i; }
}

uint256 rewardsCap            = Maths.wmul(UPDATE_CAP, totalBurned);
uint256 rewardsClaimedInEpoch = updateRewardsClaimed[curBurnEpoch];

// update total tokens claimed for updating bucket exchange rates tracker
if (rewardsClaimedInEpoch + updatedRewards_ >= rewardsCap) {
    // if update reward is greater than cap, set to remaining difference
    updatedRewards_ = rewardsCap - rewardsClaimedInEpoch;
}

// accumulate the full amount of additional rewards
updateRewardsClaimed[curBurnEpoch] += updatedRewards_;

In the else block of the function, updatedRewards_ is incremented by the _updateBucketExchangeRateAndCalculateRewards function. If updatedRewards_ is not initialized, it may result in the value being incremented from an arbitrary memory location, potentially leading to unexpected behavior or vulnerabilities in the contract.

Proof of Concept

Assume an attacker wants to manipulate the updatedRewards variable by exploiting the uninitialized memory vulnerability. The attacker can send a transaction to the contract with block.timestamp greater than curBurnTime + UPDATE_PERIOD so that the condition block.timestamp <= curBurnTime + UPDATE_PERIOD in the else block of the function evaluates to false. This will cause the updatedRewards variable to remain uninitialized.

The attacker can then call the function with an arbitrary pool_ address and an array of indexes_ that contains one or more valid bucket indexes. Since curBurnEpoch is greater than 0 and the condition block.timestamp <= curBurnTime + UPDATE_PERIOD is false, the else block of the function will be executed. In this block, the updatedRewards variable will not be initialized before being incremented by the _updateBucketExchangeRateAndCalculateRewards function.

As a result, the updatedRewards variable will contain an arbitrary value from an uninitialized memory location, which could lead to unexpected behavior in the contract. For example, if the uninitialized value is larger than the expected reward, the attacker could claim more rewards than they are entitled to. Conversely, if the uninitialized value is smaller than the expected reward, the attacker could miss out on rewards they are entitled to.

Tools Used

vscode

Recommended Mitigation Steps

The updatedRewards variable should be initialized before being used in the else block of the updateBucketExchangeRates function. This can be done by setting updatedRewards = 0 at the beginning of the else block.

Assessed type

Error

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid