Beedle - Oracle free perpetual lending
BeedleFi
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Staking.sol: Incorrect implementation of the `deposit` causes the reward distribution to not be as intended.

pengun

Summary

index is being calculated incorrectly by calling updateFor after transferFrom in the deposit function. This is affecting the distribution of rewards, causing some of the rewards to remain undistributed, as well as users to receive less or more than the intended reward.

Vulnerability Details

function update() public {
uint256 totalSupply = TKN.balanceOf(address(this)); //@audit using TKN balance
if (totalSupply > 0) {
uint256 _balance = WETH.balanceOf(address(this));
if (_balance > balance) {
uint256 _diff = _balance - balance;
if (_diff > 0) {
uint256 _ratio = _diff * 1e18 / totalSupply;
if (_ratio > 0) {
index = index + _ratio;
balance = _balance;
}
}
}
}
}

In update function sets the quantity of TKN tokens currently owned by the contract to totalSupply. In this case, totalSupply is used to calculate ratio. uint256 _ratio = _diff * 1e18 / totalSupply;

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

The deposit function performs an updateFor after performing a transferFrom, which means that the totalSupply will reflect the deposit quantity when it enters the update logic. However, the quantity deposited after the supply of weth should not be included in the reward distribution and should be excluded from totalSupply. This will cause totalSupply to be larger than the actual amount of TKN that should be rewarded, resulting in an incorrect ratio calculation.

POC:

function testScenario3_1() public {
​
// [STEP 1] staker1 deposit 1000
vm.startPrank(staker1);
stakingToken.approve(address(staking), TEST_AMOUNT);
staking.deposit(TEST_AMOUNT);
vm.stopPrank();
​
// [STEP 2] staker2 deposit 1000
vm.startPrank(staker2);
stakingToken.approve(address(staking), TEST_AMOUNT);
staking.deposit(TEST_AMOUNT);
vm.stopPrank();
​
// [STEP 3] fund 1000 WETH
vm.startPrank(funder);
weth.approve(address(staking), TEST_AMOUNT);
weth.transfer(address(staking), TEST_AMOUNT);
vm.stopPrank();
​
// [STEP 4] staker3 deposit 1000
// @audit this sould be excluded from distribution.
vm.startPrank(staker3);
stakingToken.approve(address(staking), TEST_AMOUNT);
staking.deposit(TEST_AMOUNT);
vm.stopPrank();
​
​
// [STEP 5] staker1 claim 333.33 (should be 500)
vm.startPrank(staker1);
staking.claim();
console.log("[1] staker1 claim", weth.balanceOf(staker1));
vm.stopPrank();
​
// [STEP 6] staker2 claim 333.33 (should be 500)
vm.startPrank(staker2);
staking.claim();
console.log("[2] staker2 claim", weth.balanceOf(staker2));
vm.stopPrank();
​
// [STEP 6] staker3 claim 0 (should be 0)
vm.startPrank(staker3);
staking.claim();
console.log("[3] staker3 claim", weth.balanceOf(staker3));
vm.stopPrank();
console.log("[4] weth left in staking", weth.balanceOf(address(staking)));
}

This POC shows that staker3's deposit reduces staker1,2's claim quantity.

Impact

Fee rewards are not distributed properly and some WETH is locked in the contract.

Tools Used

VS Code

Recommendations

Change the order of updateFor and transferFrom.

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

Support

FAQs

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