WETH deposited before the contract gets any staked tokens can get permanently locked

[codehawks-bot]

WETH deposited before the contract gets any staked tokens can get permanently locked

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Staking.sol#L53-L58

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Staking.sol#L57

Summary

Any WETH deposited while there are no staking tokens in the system can be permanently locked by calling claim().

Vulnerability Details

The claim() function sets the internal accounting balance of the contract to the actual WETH balance of the contract. This becomes an issue when there is a discrepancy between the balance and the internal accounting due to a WETH refill in the contract. In the case that it gets called during a case where there are no collateral tokens in the protocol the funds will be lost forever because there will not be an index increment because of them.

Consider the following PoC clearly demonstrating the issue:

https://gist.github.com/CrisCodesCrap/092f87c74221549526d816fd9e4c6033

Impact

The funds deposited at that time will be locked forever.

Tools Used

Manual Review

Recommendations

Consider adding some protection to the claim(), and how it can be called. Also consider not setting the internal accounting to the current balance, but to the old accounting amount - the claimed amount.

balance = balance - claimedAmount;

[KristianApostolov]

I believe this issue is a duplicate of #1031 as both of them cite claim()'s assignment of balance to the contract's WETH balance as the core issue.

[PatrickAlphaC]

I think you're right