An attacker can become the first depositor for a recently created YVault contract, providing a tiny amount of token by calling deposit(1) (raw values here, 1 is 1 wei, 1e18 is 1 token if it is 18 decimals). Then the attacker can directly transfer, for example, 10^6*1e18 - 1 of the token to YVault contract, effectively setting the cost of 1 of vault token to be 10^6 * 1e18 of token. The attacker will still own 100% of the remaining token pool being the only depositor.
All subsequent depositors will have their token investments rounded to 10^6 * 1e18, due to the lack of precision which initial tiny deposit caused, with the remainder divided between all current depositors, i.e. the subsequent depositors lose value to the attacker.
For example, if the second depositor brings in 1.9*10^6 * 1e18, only 1 of new JPEGDtoken to be issued, which means that 2.9*10^6 * 1e18 total token pool be divided 50/50 between the second depositor and the attacker, as each have 1 wei of the total 2 wei of JPEGDtoken, i.e. the depositor lost and the attacker gained 0.45*10^6 * 1e18 of token.
As there are no penalties to exit, the attacker can remain staked for an arbitrary time, gathering the share of all new deposits' remainder amounts.
Proof of concept
deposit() doesn't require minimum amount and mints according to the provided amount:
A minimum for deposit value can drastically reduce the economic viability of the attack. I.e. deposit() can require each amount to surpass the threshold, and then an attacker would have to provide too big direct investment to capture any meaningful share of the subsequent deposits.
An alternative is to require only the first depositor to freeze big enough initial amount of liquidity. This approach has been used long enough in Uniswap V2:
spaghettieth
commented
2 years ago
Lines of code
https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/yVault/yVault.sol#L142-L154
Vulnerability details
Impact
An attacker can become the first depositor for a recently created YVault contract, providing a tiny amount of token by calling
deposit(1)(raw values here,1is1 wei,1e18is1 tokenif it is 18 decimals). Then the attacker can directly transfer, for example,10^6*1e18 - 1of the token to YVault contract, effectively setting the cost of1of vault token to be10^6 * 1e18of token. The attacker will still own 100% of the remaining token pool being the only depositor.All subsequent depositors will have their token investments rounded to
10^6 * 1e18, due to the lack of precision which initial tiny deposit caused, with the remainder divided between all current depositors, i.e. the subsequent depositors lose value to the attacker.For example, if the second depositor brings in
1.9*10^6 * 1e18, only1of new JPEGDtoken to be issued, which means that2.9*10^6 * 1e18total token pool be divided 50/50 between the second depositor and the attacker, as each have 1 wei of the total 2 wei of JPEGDtoken, i.e. the depositor lost and the attacker gained0.45*10^6 * 1e18of token.As there are no penalties to exit, the attacker can remain staked for an arbitrary time, gathering the share of all new deposits' remainder amounts.
Proof of concept
deposit() doesn't require minimum amount and mints according to the provided amount:
https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/yVault/yVault.sol#L142-L154
When YVault is new the balance() is just initially empty contract balance:
https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/yVault/yVault.sol#L75-L79
Any deposit lower than total attacker's stake will be fully stolen from the depositor as
0vault tokens will be issued in this case.References
The issue is similar to the
TOB-YEARN-003one of the Trail of Bits audit of Yearn Finance:https://github.com/yearn/yearn-security/tree/master/audits/20210719_ToB_yearn_vaultsv2
Recommended Mitigation Steps
A minimum for deposit value can drastically reduce the economic viability of the attack. I.e.
deposit()can require each amount to surpass the threshold, and then an attacker would have to provide too big direct investment to capture any meaningful share of the subsequent deposits.An alternative is to require only the first depositor to freeze big enough initial amount of liquidity. This approach has been used long enough in Uniswap V2:
https://github.com/Uniswap/v2-core/blob/master/contracts/UniswapV2Pair.sol#L119-L121