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

Depositing in the Staking contract does not increase user claimable funds

Summary

Depositing in the Staking contract does not increase user claimable funds due to an error with claimable internal accounting.

Vulnerability Details

The Staking::deposit function:

function deposit(uint _amount) external {
TKN.transferFrom(msg.sender, address(this), _amount);
updateFor(msg.sender);
balances[msg.sender] += _amount;
}

Function:

  1. transfers tokens from sender to staking contract

  2. updates internal accounting including claimable funds

  3. updates the internal accounting variable that indicates that the user has further increased his balance

Because 3 is made before 2, user is set as having no claimable rewards.

Coded POC:

Create a file test\Staking.t.sol with the following code:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
import "../src/Staking.sol";
import "../src/Beedle.sol";
import "forge-std/Test.sol";
contract WETH9 is ERC20 {
constructor () ERC20("WETH", "WETH") {}
function name() public pure override returns (string memory) {
return "Test ERC20";
}
function symbol() public pure override returns (string memory) {
return "WETH";
}
function mint(address _to, uint256 _amount) public {
_mint(_to, _amount);
}
}
contract StakingTest is Test {
Beedle public beedle;
WETH9 public weth;
Staking public staking;
address public user1 = makeAddr("user1");
address public user2 = makeAddr("user2");
address public user3 = makeAddr("user3");
function setUp() public {
beedle = new Beedle();
weth = new WETH9();
staking = new Staking(address(beedle), address(weth));
beedle.mint(user1, 1000e18);
beedle.mint(user2, 1000e18);
beedle.mint(user3, 1000e18);
vm.startPrank(user1);
beedle.approve(address(staking), 1000e18);
staking.deposit(1000e18);
vm.stopPrank();
vm.startPrank(user2);
beedle.approve(address(staking), 1000e18);
staking.deposit(1000e18);
vm.stopPrank();
vm.startPrank(user3);
beedle.approve(address(staking), 1000e18);
staking.deposit(1000e18);
vm.stopPrank();
weth.mint(address(staking), 100e18);
}
function testDepositDoesNotIncreaseClaimable() public {
address user4 = makeAddr("user4");
// imitate a flash loan being called
beedle.mint(user4, 1000e18);
vm.startPrank(user4);
beedle.approve(address(staking), 10000e18);
console.log("staking.claimable(user4) before deposit:", staking.claimable(user4));
assertEq(staking.claimable(user4), 0);
staking.deposit(1000e18);
assertEq(staking.claimable(user4), 0);
console.log("staking.claimable(user4) after deposit: ", staking.claimable(user4));
vm.stopPrank();
}
}

and run it via forge test --match-test testDepositDoesNotIncreaseClaimable -vv. Output will be:

Running 1 test for test/Staking.t.sol:StakingTest
[PASS] testDepositDoesNotIncreaseClaimable() (gas: 185233)
Logs:
staking.claimable(user4) before deposit: 0
staking.claimable(user4) after deposit: 0

Impact

Users have no incentive to deposit tokens into the staking contract as they will get nothing in return.

Tools Used

Manual review

Recommend Mitigation

Move the Balance update before calling updateFor. Example change:

function deposit(uint _amount) external {
TKN.transferFrom(msg.sender, address(this), _amount);
balances[msg.sender] += _amount;
updateFor(msg.sender);
}

Support

FAQs

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