`Controller.setCap` sets wrong vault balance

[code423n4]

Handle

cmichel

Vulnerability details

The Controller.setCap function sets a cap for a strategy and withdraws any excess amounts (_diff). The vault balance is decreased by the entire strategy balance instead of by this _diff:

// @audit why not sub _diff?
_vaultDetails[_vault].balance = _vaultDetails[_vault].balance.sub(_balance);

Impact

The _vaultDetails[_vault].balance variable does not correctly track the actual vault balances anymore, it will usually underestimate the vault balance. This variable is used in Controller.balanceOf(), which in turn is used in Vault.balance(), which in turn is used to determine how many shares to mint / amount to receive when redeeming shares. If the value is less, users will lose money as they can redeem fewer tokens. Also, an attacker can deposit and will receive more shares than they should receive. They can then wait until the balance is correctly updated again and withdraw their shares for a higher amount than they deposited. This leads to the vault losing tokens.

Recommended Mitigation Steps

Sub the _diff instead of the balance: _vaultDetails[_vault].balance = _vaultDetails[_vault].balance.sub(_diff);

[Haz077]

Already fixed in code-423n4/2021-09-yaxis#1

[GalloDaSballo]

Finding is valid, has been mitigated by sponsor as of 14 days ago