earn results in decreasing share price

[code423n4]

Handle

jonah1005

Vulnerability details

Impact

For a dai vault that pairs with NativeStrategyCurve3Crv, every time earn() is called, shareholders would lose money. (about 2%)

There're two issues involved. The Vault contract and the controller contract doesn't handle the price difference between the want token and other tokens.

Vault.sol#L293 When a vault calculates its value, it sums up all tokens balance. Controller.sol#L410-L436 However, when the controller calculates vaults' value, it only adds the amount of strategy.want it received. (in this case, it's t3crv).

Under the current design, users who deposit dai to the vault would not get yield. Instead, they would keep losing money. I consider this a high-risk issue

Proof of Concept

I trigger the bug with the following web3.py script:

previous_price = vault.functions.getPricePerFullShare().call()
vault.functions.available(dai.address).call()
vault.functions.earn(dai.address, strategy.address).transact()
current_price = vault.functions.getPricePerFullShare().call()
print(previous_price)
print(current_price)

Tools Used

Hardhat

Recommended Mitigation Steps

The protocol should decide what the balance sheet in each contract stands for and make it consistent in all cases. Take, for example, if _vaultDetails[_vault].balance; stands for the amount of 'want' token the vault owns, there shouldn't exist two different want in all the strategies the vault has. Also, when the vault queries controllers function balanceOf(), they should always multiply it by the price.

[gpersoon]

I think this is also related to the underlying problem that all coins are assumed to have the same value.
See also #2, #8 and #158

[GalloDaSballo]

Agree with wardens finding and acknowledge it's similitude with other issues

Personally this is a different vulnerability that can be solved by solving the same underlying problem

Marking this as unique finding as it's a specific exploit the protocol could face