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

Issues with `deposit` function of the Staking contract results in underpaying rewards & rewards in staking contract prior to first staker being lost

toshii

Summary

There is currently an issue with the deposit function of the Staking contract which will result in two issues. The first is: all of the reward token which is inside the Staking contract prior to the first staker will be irretrievable. This means those tokens will be trapped inside the Staking contract and will never be awarded to any user. The second issue is: less rewards than expected will be paid out for stakers. For example, if a staker has staked the entire balance of TKN in the contract, they will still not receive all the rewards, which is invalid logic.

Vulnerability Details

Both issues have the same source, so I will highlight the issue of lost reward tokens, when there is WETH in the Staking contract prior to the first staker having staked.

Let's consider the case where the WETH (rewards token) balance of the staking contract is > 0. The first staker in this contract will call deposit which is defined as follows:

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

At this point in time, although the TKN balance of this contract is now > 0, the balance for msg.sender will still be 0. This calls updateFor which is defined as follows:

function updateFor(address recipient) public {
update();
uint256 _supplied = balances[recipient];
if (_supplied > 0) {
uint256 _supplyIndex = supplyIndex[recipient];
supplyIndex[recipient] = index;
uint256 _delta = index - _supplyIndex;
if (_delta > 0) {
uint256 _share = _supplied * _delta / 1e18;
claimable[recipient] += _share;
}
} else {
supplyIndex[recipient] = index;
}
}

This function then first calls update, which is defined as follows:

function update() public {
uint256 totalSupply = TKN.balanceOf(address(this));
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;
}
}
}
}
}

As mentioned previously, the TKN supply is > 0, while _balance is also greater than balance, as in this first call we have WETH in this contract, which has not yet been accounted for (balance = _balance). In this function, _ratio > 0, meaning index will equal the WETH supply prior to the first deposit divided by the amount of TKN deposited by the first staker.

Back in the updateFor function, _supplied will be 0, as this mapping for the user has no yet been updated with the amount of TKN they staked. This then sets the supplyIndex for the user with: supplyIndex[recipient] = index;, which is effectively the reward debt for the user. Since this user and any subsequent user will get paid out rewards based on the following logic:

uint256 _delta = index - _supplyIndex;
if (_delta > 0) {
uint256 _share = _supplied * _delta / 1e18;
claimable[recipient] += _share;
}

this initial amount of WETH will never get paid out to any of the stakers, and will instead be trapped inside the contract.

Impact

All of the reward token which is inside the Staking contract prior to the first staker will be irretrievable, meaning direct loss of rewards for stakers. There is also an issue of stakers getting less rewards than they should.

Tools Used

Manual review

Recommendations

The deposit function of the Staking contract should be updated to the following:

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.