Summary
The rewards index is diluted by subsequent stakers, resulting in fewer rewards for previous stakers and thus unutilized and not claimable WETH tokens, which remain stuck in the Staking contract.
Vulnerability Details
The call to updateFor in line 40 of the deposit function in the Staking contract after transferring the staking TKN tokens from the caller (msg.sender) to the contract (address(this)) is incorrectly positioned, meaning, the call should happen before the TKN token transfer in line 39.
Having the updateFor function call after the token transfer results in diluting the rewards index due to the increased totalSupply denominator when calculating index in line 68. Consequently, not all topped-up WETH tokens can be utilized as staking rewards, resulting in stuck WETH tokens in the Staking contract.
Staking.sol#L40
38: function deposit(uint _amount) external {
39: TKN.transferFrom(msg.sender, address(this), _amount);
40: @> updateFor(msg.sender);
41: balances[msg.sender] += _amount;
42: }
Staking.sol#L68
61: function update() public {
62: uint256 totalSupply = TKN.balanceOf(address(this));
63: if (totalSupply > 0) {
64: uint256 _balance = WETH.balanceOf(address(this));
65: if (_balance > balance) {
66: uint256 _diff = _balance - balance;
67: if (_diff > 0) {
68: uint256 _ratio = _diff * 1e18 / totalSupply;
69: if (_ratio > 0) {
70: index = index + _ratio;
71: balance = _balance;
72: }
73: }
74: }
75: }
76: }
Test case demonstrating the dilution:
The following test case demonstrates how the first staker, Bob (bob), deposits 500e18 tokens and receives 10e18 WETH tokens as staking rewards (the total amount of WETH token rewards at that time). Following, 10e18 additional WETH tokens are topped-up as rewards.
However, the second staker, Alice (alice), deposits 500e18 tokens and due to the above explained issue, Bob's claimable rewards, which should be 20e18 WETH tokens, are only 15e18 WETH tokens. The remaining 5e18 WETH tokens are stuck in the Staking contract.
pragma solidity ^0.8.13;
import "./../src/Staking.sol";
import "forge-std/Test.sol";
import {ERC20} from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
contract WETHMock is ERC20 {
constructor() ERC20("WETHMock", "WETH") {}
function mint(address account, uint256 amount) external {
_mint(account, amount);
}
function burn(address account, uint256 amount) external {
_burn(account, amount);
}
}
contract TKNMock is ERC20 {
constructor() ERC20("TKNMock", "TKN") {}
function mint(address account, uint256 amount) external {
_mint(account, amount);
}
function burn(address account, uint256 amount) external {
_burn(account, amount);
}
}
contract ContractTest is Test {
constructor() {}
function testDeposit() public {
WETHMock weth = new WETHMock();
TKNMock tkn = new TKNMock();
Staking staking = new Staking(address(tkn), address(weth));
tkn.mint(address(this), 1002e18);
weth.mint(address(this), 100e18);
address bob = vm.addr(1);
address alice = vm.addr(2);
tkn.transfer(bob, 501e18);
tkn.transfer(alice, 501e18);
vm.prank(bob);
tkn.approve(address(staking), type(uint256).max);
vm.prank(alice);
tkn.approve(address(staking), type(uint256).max);
vm.prank(bob);
staking.deposit(500e18);
weth.transfer(address(staking), 10e18);
staking.updateFor(bob);
assertEq(staking.balances(bob), 500e18);
assertEq(staking.claimable(bob), 10e18);
assertEq(staking.supplyIndex(bob), 2e16);
weth.transfer(address(staking), 10e18);
assertEq(weth.balanceOf(address(staking)), 20e18);
vm.prank(alice);
staking.deposit(500e18);
assertEq(staking.balances(alice), 500e18);
assertEq(staking.claimable(alice), 0);
assertEq(staking.supplyIndex(alice), 2e16 + 1e16);
staking.updateFor(bob);
assertEq(staking.balances(bob), 500e18);
assertEq(staking.claimable(bob), 15e18);
assertEq(staking.supplyIndex(bob), 2e16 + 1e16);
}
}
Copy paste the test case into a new Solidity file next in the test folder and run with:
forge test -vv --match-test "testDeposit"
Impact
Stakers receive fewer staking rewards than expected, resulting in stuck WETH tokens in the Staking contract.
Tools Used
Manual Review
Recommendations
Consider moving the updateFor call before the TKN token transfer in line 39.