Beedle - Oracle free perpetual lending

Summary

In the provided Staking contract's deposit function, the sequence of operations results in the first depositor being unable to receive any rewards. The issue arises due to the order of operations in the deposit function, where the updateFor function is called before the user's balance is updated. This, in combination with conditions in the update and updateFor functions, results in the first depositor's shares not being added.

Vulnerability Details

In the deposit function, the contract calls updateFor(msg.sender) before adding the deposited amount to balances[msg.sender]. The updateFor function, in turn, calls the update function. The update function has a condition to only perform an update if _balance > balance. However, for the first depositor, _balance would be zero, as there are no rewards in the contract yet. Similarly, balance would also be zero, as it's defaulted to zero in the contract. So, no update would be made to the contract's index or balance in this case.

When execution returns to the updateFor function, it also has a condition to update only if (_supplied > 0). But for the first depositor, _supplied would also be zero because the balance is updated in the deposit function only after calling updateFor. As a result, the execution wouldn't reach the line where the user's shares are added, causing the first depositor to not receive any rewards.

Here is the deposit() function:

Impact

The first depositor possibly missing out on rewards can have a significant impact on the fairness and appeal of the staking system. It also creates a potentially unintended imbalance in reward distribution.

Tools Used

Manual Audit

Recommended Mitigation

Pretty interesting case, but the recommended solution imo is to move the updateFor(msg.sender) function call to after the user's balance is updated in the deposit function. This ensures that the user's balance and the state of the contract are updated correctly before attempting to distribute rewards.

The corrected deposit function could look like this:

With this change, I think the first depositor should now be correctly accounted for.