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

Deposit backrunning a WETH transaction to the contract with a deposit locks a fraction of those funds

KrisApostolov

Summary

A CEI violation and bad state update sequence in Staking.sol's deposit() allows for the locking of reward WETH.

Vulnerability Details

The staking protocol gets WETH used for rewarding stakers by it getting sent through an ERC20 transfer. The issue arises due to deposit() first taking in the staking tokens instead of first updating the balances.

function deposit(uint _amount) external {
TKN.transferFrom(msg.sender, address(this), _amount); // @audit tokens getting to the contract first
updateFor(msg.sender);
balances[msg.sender] += _amount;
}

This leads to an offset in the delta calculation in the update right after the transfer.

// @audit totalSupply is now bigger because of the deposit of user tokens just before that
uint256 _ratio = _diff * 1e18 / totalSupply;

This will make the protocol calculation lock a specific amount of WETH for the deposited amount of staking tokens as well.

Here is a PoC demonstrating the issue at hand:
https://gist.github.com/CrisCodesCrap/38cb06e3864ace97734a1d86a61de733

Impact

The delta ratio calculation will be offset, which will lead to some amount staking reward loss for every staker in the contract.

Tools Used

Manual Review

Recommendations

Consider keeping the balances of tokens in the protocol only as accounting variables, not as the real values the contract owns. A good example would the how the WETH balance in the contract is handled. Also, consider following the CEI pattern in this function and in every other one in the protocol. https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html

Support

FAQs

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