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

Any user can stuck WETH in `Staking.sol`

Summary

Any user who withdraws before calling claim can have WETH stuck forever in Staking.sol.

Vulnerability Details

Due to the nature of how shares are calculated, and not claimed when someone calls withdraw, there is a possibility that any user maliciously, unknowingly or accidentally can withdraw before claiming. And when he withdraws before claiming the WETH he quarried will remain stuck in the contract forever.

This issue arises from 2 different things:

  • WETH is not claimed on withdraw

  • Shares are only calculated with an increase on already accumulated balances

First one is simple, so lets focus on the latter one. When update is called,the difference of the 2 balances is only added. If there was already a balance (like the unclaimed one) it will not be included in the diff, thus will not account for any increase.

uint256 _diff = _balance - balance;
if (_diff > 0) {
uint256 _ratio = _diff * 1e18 / totalSupply;
if (_ratio > 0) {
index = index + _ratio;
balance = _balance;

And because updateFor only adds the index extracted from update it will also not be able to add the amount that was not claimed.

Now here are some examples of how this can happen:

  • User accidentally withdraws without claiming

  • Tho really, a reorg can make it so only withdraw executes on the long chain

  • Claim stays in the mem pool for a long time and when user the calls withdraw it gets executed first

POC
contract Staker is Test {
Staking public stakingContract;
TERC20 public weth;
TERC20 public tkn;
address public user1 = address(0x1);
address public user2 = address(0x2);
address public owner = address(0x3b);
function setUp() public {
vm.label(user1, "user1");
vm.label(user2, "user1");
vm.label(owner, "user1");
weth = new TERC20();
tkn = new TERC20();
weth.mint(address(owner), 50e18);
tkn.mint(address(user1), 10e18);
tkn.mint(address(user2), 10e18);
stakingContract = new Staking(address(tkn),address(weth));
}
function test_stuck_tokens() public {
vm.startPrank(user1);
tkn.approve(address(stakingContract),100e18);
stakingContract.deposit(10e18);
vm.stopPrank();
vm.startPrank(user2);
tkn.approve(address(stakingContract),100e18);
stakingContract.deposit(10e18);
vm.stopPrank();
vm.prank(owner);
weth.transfer(address(stakingContract),20e18);
vm.prank(user1);
stakingContract.withdraw(10e18);
vm.prank(owner);
weth.transfer(address(stakingContract),10e18);
vm.prank(user2);
stakingContract.claim();
console.log("WETH: ",weth.balanceOf(address(stakingContract))/1e18);
console.log("TKN: ",tkn.balanceOf(address(stakingContract))/1e18);
}
}

Impact

WETH can become stuck forever.

Tools Used

Manual review

Recommendations

Claim shares when someone withdraws. Here is some pseudo code:

function withdraw(uint _amount) external {
updateFor(msg.sender);
+ claim();
balances[msg.sender] -= _amount;
TKN.transfer(msg.sender, _amount);
}

And you would need to make claim public, or even better - Make an internal claim that is called by main claim and withdraw.

Support

FAQs

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