Vault's deposit
function rounds to its disadvantage in divisor
Description
To determine how many of the vault's shares a caller depositing will receive, the vault's deposit
function calculates the value in token1 that the vault is currently worth (lines 178 and 179)
// How much of wants() do we have in token 1 equivalents;
uint256 token1EquivalentBalance = (_bal0 * price / PRECISION) + _bal1;
as well as the value in token1 that the caller's deposit is worth (line 173):
shares = _amount1 + (_amount0 * price / PRECISION);
Note that as all values are unsigned, the division by PRECISION
will always round down. The number of shares the caller receives is then obtained by a calculation in which the value of shares
as above is divided by token1EquivalentBalance
(line 180).
shares = shares * _totalSupply / token1EquivalentBalance;
When calculating the amount of shares minted for a caller, it is best to always round in the vault's favor, to avoid any manipulation that could result in an attacker receiving more shares than they should. For a quotient like here, this means the dividend should be rounded down and the divisor rounded up. Rounding down in the calculation of token1EquivalentBalance
is instead to the caller's advantage, as it reduces the divisor, hence increasing the quotient.
Impact
A caller might receive more shares than they should.
Recommendations
Consider rounding up in the calculation of token1EquivalentBalance
, for example by replacing line 179 by the following line.
uint256 token1EquivalentBalance = (((_bal0 * price) + PRECISION - 1) / PRECISION) + _bal1;
Remediation
This issue has been acknowledged by Beefy, and a fix was implemented in commit efb9e59e↗.