Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)
Description
Brief/Intro
The Staking.sol inherits ERC4626 from solady. This ERC4626 contract uses the ERC20 balanceOf(address(this)) function to keep record of the total assets in the contract. The totalAssets() function of Staking.sol can be increased by anyone by direct donation.
This totalAssets() function is used in shares minting during deposit and the attacker's aim will be to cause the victim to mint zero shares losing their deposits.
Vulnerability Details
The main attack is to cause victims to mint zero shares during deposits. Zero shares means the victim cannot withdraw their deposit.
The totalAssets() is the denominator in the convertToShares function of ERC4626. An attacker can frontrun a victim's deposit with a direct transfer of Long token to the Staking.sol contract increasing totalAssets() without changing totalSupply().
Due to solidity rounding, a victim deposit can round down to zero if the totalAssets() is made large enough with the donation. shares = userAsset × totalSupply / totalAssets.
This issue compounds if the next depositor deposits asset amount less than the current total amount. The Staking Contract takes in the asset and mints zero shares again. Now totalAssets has increased again while totalSupply remains the same; subsequent users who deposit less than the new totalAssets also mint zero shares.
Example snippet from solady ERC4626:
Issue Scenario
1
Step
Staking.sol is deployed
2
Step
Bob (victim) tries to deposit 100 Long tokens
3
Step
Alice (attacker) frontruns Bob to directly transfer 101 Long tokens to Staking.sol
4
Step
Bob's deposit is executed with 100 × 1 / 101 = 0
5
Step
Zero shares are minted to Bob making Bob lose his 100 Long tokens
6
Step
John (another victim) deposits 150 Long tokens and gets 150 × 1 / 201 = 0
As total assets keep growing while totalSupply remains the same, repeated small donations or repeated deposits smaller than the current totalAssets can cause many users to mint zero shares and lose funds.
Impact Details
Causing loss of funds to the first depositors by making them mint zero shares.
Recommendation: Consider overriding the totalAssets(...) function of the ERC4626 then keep record of the total assets with a storage variable which will be updated during deposit(), withdraw() and distributeRewards().
Proof of Concept
1
Test setup
Copy and paste the test below into the staking.test.sol file in the 'Staking features' test suite
Run yarn test
2
Observed effect
This demonstrates a donation attack that causes the first depositor to lose their Long token deposit amount.