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

2 stars 0 forks source link

Users may permanently lose a portion of rewards when claiming rewards if the contract balance is insufficient #361

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#L811-L821

Vulnerability details

Impact

The current implementation of the _transferAjnaRewards function in the smart contract may result in a permanent loss of rewards for users if the contract balance of ajnaToken is insufficient to cover the total rewards earned. In such a scenario, the user will only receive the remaining ajnaToken balance as a reward, while the unpaid portion will be permanently lost.

Proof of Concept

The relevant code snippet can be found in the _transferAjnaRewards function:

uint256 ajnaBalance = IERC20(ajnaToken).balanceOf(address(this));
if (rewardsEarned_ > ajnaBalance) rewardsEarned_ = ajnaBalance;

if (rewardsEarned_ != 0) {
    // transfer rewards to sender
    IERC20(ajnaToken).safeTransfer(msg.sender, rewardsEarned_);
}

The function checks if the rewardsEarned_ is greater than the contract's ajnaToken balance (ajnaBalance). If so, the rewardsEarned_ is set equal to the remaining balance, effectively causing the user to permanently lose the unpaid portion of the rewards.

Tools Used

N/A

Recommended Mitigation Steps

To address this issue, it is recommended to keep track of the unpaid rewards separately, rather than permanently losing them due to an insufficient contract balance. The _transferAjnaRewards function can be modified to store the unpaid rewards for each user, allowing the possibility of future claims when the contract balance is replenished.

An example of a modified _transferAjnaRewards function could look like this:

function _transferAjnaRewards(uint256 rewardsEarned_) internal {
    // check that rewards earned isn't greater than remaining balance
    // if remaining balance is greater, set to remaining balance
    uint256 ajnaBalance = IERC20(ajnaToken).balanceOf(address(this));
    uint256 payableRewards = rewardsEarned_;
    uint256 unpaidRewards = 0;

    if (rewardsEarned_ > ajnaBalance) {
        payableRewards = ajnaBalance;
        unpaidRewards = rewardsEarned_ - ajnaBalance;
        // Store unpaid rewards for the user (e.g., using a mapping)
        userUnpaidRewards[msg.sender] += unpaidRewards;
    }

    if (payableRewards != 0) {
        // transfer rewards to sender
        IERC20(ajnaToken).safeTransfer(msg.sender, payableRewards);
    }
}

By implementing this change, users will be able to claim their unpaid rewards at a later time when the contract balance is sufficient.

Assessed type

Token-Transfer

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

c4-sponsor commented 1 year ago

MikeHathaway marked the issue as sponsor confirmed

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory

c4-judge commented 1 year ago

Picodes marked issue #251 as primary and marked this issue as a duplicate of 251