20,000 USDC
View results
Submission Details
Severity: high
Valid

Staking deposit function transferFrom first and then updateFor causing partial reward get stuck in the contract forever

Summary

Staking deposit function transferFrom first and then updateFor, transferFrom will increase totalSupply and decrease the reward index, which resulted in some of the rewards never being claimed, stuck in the contract.

Vulnerability Details

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
import "forge-std/Test.sol";
import "solady/src/tokens/ERC20.sol";
import "../src/Staking.sol";
contract SERC20 is ERC20 {
function name() public pure override returns (string memory) {
return "Test ERC20";
}
function symbol() public pure override returns (string memory) {
return "TERC20";
}
function mint(address _to, uint256 _amount) public {
_mint(_to, _amount);
}
}
contract StakingTest is Test {
SERC20 st;
SERC20 weth;
Staking staking;
function setUp() public {
st = new SERC20();
weth = new SERC20();
staking = new Staking(address(st), address(weth));
}
function testDeposit() public {
address alice = makeAddr("Alice");
address bob = makeAddr("Bob");
deal(address(weth), address(staking), weth.balanceOf(address(staking)) + 1 ether);
deal(address(st), address(alice), 2 ether);
deal(address(st), address(bob), 2 ether);
vm.startPrank(bob);
st.approve(address(staking), 2 ether);
staking.deposit(1 ether);
staking.claim();
vm.stopPrank();
// @audit Bob won't be able to claim any rewards, which are stuck in the contract
assertEq(weth.balanceOf(bob), 0);
vm.roll(100);
deal(address(weth), address(staking), weth.balanceOf(address(staking)) + 1 ether);
vm.startPrank(alice);
st.approve(address(staking), 2 ether);
staking.deposit(1 ether);
staking.claim();
vm.stopPrank();
vm.startPrank(bob);
staking.claim();
vm.stopPrank();
// @audit Alice won't be able to claim any rewards
assertEq(weth.balanceOf(alice), 0);
// @audit Bob can only gets half the reward
assertEq(weth.balanceOf(bob), 0.5 ether);
vm.roll(200);
vm.startPrank(alice);
staking.claim();
vm.stopPrank();
vm.startPrank(bob);
staking.claim();
vm.stopPrank();
// @audit The rest of the reward cannot be claimed and is locked in the contract forever
assertEq(weth.balanceOf(alice), 0);
assertEq(weth.balanceOf(bob), 0.5 ether);
vm.roll(300);
deal(address(weth), address(staking), weth.balanceOf(address(staking)) + 1 ether);
vm.startPrank(alice);
staking.deposit(1 ether);
staking.claim();
vm.stopPrank();
vm.startPrank(bob);
staking.claim();
vm.stopPrank();
// @audit Alice and Bob can only get 1/3 reward because of totalSupply
uint256 reward = 1 ether;
reward /= 3;
assertEq(weth.balanceOf(alice), reward);
assertEq(weth.balanceOf(bob), 0.5 ether + reward);
}
}

The above contract shows the effect of a change in totalSupply resulting from executing transferFrom first on the distribution of rewards. There are three main scenarios:

  1. When there is no staking user, the transferred rewards are permanently stuck in the contract

  2. If a staking user not claiming, then another user deposits with a totalSupply change, the staking user's reward will be halved and the remaining stuck in the contract

  3. If both users are staking the same amount and deposit the same amount again, each user gets 1/3 and the remaining stuck in the contract

Impact

Staking deposit function transferFrom first and then updateFor may causing partial reward get stuck in the contract forever.

Tools Used

Foundry

Recommendations

Call updateFor first and then transferFrom

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.