code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

High impact, because it implies loss of funds for users of the protocol.

Proof of Concept

The comparison between the rewardsEarned and ajnaBalance in _transferAjnaRewards is suppose to ensure that rewardsEarned_ is less than remaining ajnaBalance before setting the rewardsEarned to the ajnaBalance. If the rewards Earned is greater than the remaining balance, it means the bucket and by extension the pool will owe the user and subsequent calls to _transferAjnaRewards will push the bucket into debt. That said, this function falls short of handling the correct logic and rewards will not be paid out to users leading to loss of funds.

    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));
        if (rewardsEarned_ > ajnaBalance) rewardsEarned_ = ajnaBalance;

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

Tools Used

VS code

Recommended Mitigation Steps

Expected behaviour

evaluate a condition where the rewardsEarned_ is actually lesser than the ajnaBalance and thus rewardsEarned is set to ajnaBalance.

Write the condition properly as shown below

    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));
        if (rewardsEarned_ < ajnaBalance) rewardsEarned_ = ajnaBalance;

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

Assessed type

Context

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #361