Skip to content

Latest commit

 

History

History
192 lines (152 loc) · 7.58 KB

089.md

File metadata and controls

192 lines (152 loc) · 7.58 KB

Lone Wintergreen Rattlesnake

High

Permanent Reward Loss Due to Asymmetric Excluded Amount Updates in Pause States

Summary

A critical vulnerability has been identified in the TokenRewards contract's reward distribution logic. The issue affects the handling of excluded rewards for paused tokens during share modifications, potentially leading to permanent loss of rewards for users. The vulnerability stems from an inconsistency in how paused tokens are handled between two key functions: In _resetExcluded:

function _resetExcluded(address _wallet) internal {
    for (uint256 _i; _i < _allRewardsTokens.length; _i++) {
        address _token = _allRewardsTokens[_i];
        rewards[_token][_wallet].excluded = _cumulativeRewards(_token, shares[_wallet], true);
    }
}

In _distributeReward:

function _distributeReward(address _wallet) internal {
    if (shares[_wallet] == 0) {
        return;
    }
    for (uint256 _i; _i < _allRewardsTokens.length; _i++) {
        address _token = _allRewardsTokens[_i];
        if (REWARDS_WHITELISTER.paused(_token)) {
            continue;
        }
        // ... reward distribution logic
    }
}

The vulnerability arises from an inconsistency in how excluded rewards are handled between paused and active tokens during share modifications. When tokens are paused: _resetExcluded updates excluded amounts for ALL tokens during share changes, including paused tokens _distributeReward skips updating excluded amounts for paused tokens during reward distribution This asymmetry leads to artificially inflated excluded amounts for paused tokens

Root Cause

_resetExcluded updates excluded rewards for ALL tokens, including paused ones _distributeReward skips paused tokens When calculating unpaid rewards:

function getUnpaid(address _token, address _wallet) public view returns (uint256) {
    uint256 earnedRewards = _cumulativeRewards(_token, shares[_wallet], false);
    uint256 rewardsExcluded = rewards[_token][_wallet].excluded;
    if (earnedRewards <= rewardsExcluded) {
        return 0;
    }
    return earnedRewards - rewardsExcluded;
}

The excluded amount will always be greater than or equal to earned rewards due to rounding up in _resetExcluded, resulting in zero rewards.

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

Users permanently lose rewards that accumulated while tokens were paused. Could result in significant financial losses depending on:

  • Duration of token pause
  • Amount of rewards accumulated
  • Frequency of share modifications

PoC

Update the MockRewardWhiteliste.sol contract to correctly mimick the contract:

contract MockRewardsWhitelister {
    bool private _paused;
    mapping(address => bool) private _whitelist;
    mapping(address => bool) private _isPaused;

    function whitelist(address token) public view returns (bool) {
        return _whitelist[token];
    }

    function setWhitelist(address token, bool status) public {
        _whitelist[token] = status;
    }

    function paused(address _token) public view returns (bool) {
        return _isPaused[_token];
    }

    function setTokenPaused(address _token, bool __isPaused) public {
        _isPaused[_token] = __isPaused;
    }

    function setPaused(bool paused_) public {
        _paused = paused_;
    }
}

run test: forge test --match-contract TokenRewardsTest --match-test testPausedTokenExcludedRewardsDiscrepancy -vvv --fork-url ""

POC:

    function testPausedTokenExcludedRewardsDiscrepancy() public {
        // Setup
        rewardsWhitelister.setWhitelist(address(rewardsToken), true);
        rewardsWhitelister.setWhitelist(address(secondaryRewardToken), true);

        vm.startPrank(address(trackingToken));
        tokenRewards.setShares(user1, 100e18, false);
        vm.stopPrank();

        uint256 depositAmount = 100e18;
        tokenRewards.depositRewards(address(secondaryRewardToken), depositAmount);

        // Record initial unpaid rewards
        uint256 initialUnpaid = tokenRewards.getUnpaid(address(secondaryRewardToken), user1);
        assertEq(initialUnpaid, depositAmount, "Initial unpaid rewards should match deposit amount");

        rewardsWhitelister.setTokenPaused(address(secondaryRewardToken), true);

        tokenRewards.depositRewards(address(secondaryRewardToken), depositAmount);

        uint256 unpaidAfterSecondDepositRewards = tokenRewards.getUnpaid(address(secondaryRewardToken), user1);
        assertEq(unpaidAfterSecondDepositRewards, depositAmount * 2, "Unpaid rewards should not change while token is paused");

        // get user 1 balance of secondary token
        uint256 user1Balance = secondaryRewardToken.balanceOf(user1);
        assertEq(user1Balance, 0, "User 1 balance of secondary token should be 0");

        vm.startPrank(address(trackingToken));
        tokenRewards.setShares(user1, 150e18, false); // Increase shares
        vm.stopPrank();

        uint256 user1BalanceAfterSetShareAndTokenPause = secondaryRewardToken.balanceOf(user1);
        // This shows user 1 did not receive any rewards because token is paused
        assertEq(user1BalanceAfterSetShareAndTokenPause, 0, "User 1 balance of secondary token should be 0"); 

        initialUnpaid = tokenRewards.getUnpaid(address(secondaryRewardToken), user1);
        // This assertion should fail because _resetExcluded updated the excluded amount
        // while the token was paused, but _distributeReward skipped it
        assertEq(initialUnpaid, 0, "Should have unpaid rewards after set share");
        
        // token deposit
        tokenRewards.depositRewards(address(secondaryRewardToken), depositAmount);
        uint256 balanceOfTokenReward = secondaryRewardToken.balanceOf(address(tokenRewards));
        assertEq(balanceOfTokenReward, depositAmount * 3, "Token reward balance should be 0");

        rewardsWhitelister.setTokenPaused(address(secondaryRewardToken), false);

        uint256 finalUnpaid = tokenRewards.getUnpaid(address(secondaryRewardToken), user1);
        // This assertion should fail because _resetExcluded updated the excluded amount
        // while the token was paused, but _distributeReward skipped it
        assertGt(balanceOfTokenReward, finalUnpaid, "Should have unpaid rewards after unpausing");
    }

Result

[⠊] Compiling...
[⠢] Compiling 1 files with Solc 0.8.28
[⠆] Solc 0.8.28 finished in 1.08s
Compiler run successful!

Ran 1 test for test/TokenRewards.t.sol:TokenRewardsTest
[PASS] testPausedTokenExcludedRewardsDiscrepancy() (gas: 397500)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 16.57s (6.04ms CPU time)

Ran 1 test suite in 16.96s (16.57s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Mitigation

Modify _resetExcluded to skip paused tokens:

function _resetExcluded(address _wallet) internal {
    for (uint256 _i; _i < _allRewardsTokens.length; _i++) {
        address _token = _allRewardsTokens[_i];
        if (REWARDS_WHITELISTER.paused(_token)) {
            continue;
        }
        rewards[_token][_wallet].excluded = _cumulativeRewards(_token, shares[_wallet], true);
    }
}