Vault: Withdrawal amount isn't un-normalized

[code423n4]

Handle

hickuphh3

Vulnerability details

Impact

In withdraw(), the withdrawal amount is the proportion of the normalized amounts of all the tokens in the vault and its strategies. However, this amount isn't un-normalized to the output token's decimals, thus leading to an erroneous token amount being withdrawn.

Proof of Concept

We take USDC as an example, since it has 6 decimals.

1. deposit(USDC, 1000e6): Deposit 1000 USDC into the vault. The user receives 1000 shares.

2. Attempt a full withdrawal withdraw(1000e18, USDC)

   _amount
   = (balance().mul(_shares)).div(totalSupply());
   = (1000e18) * 1000e18 / 1000e18
   = 1000e18

The expected _amount is 1000e6 as per the deposited amount. While the withdrawal will technically work because of the following lines:

if (_balance < _amount) {
  IController _controller = IController(manager.controllers(address(this)));
  uint256 _toWithdraw = _amount.sub(_balance);
  if (_controller.strategies() > 0) {
      _controller.withdraw(_output, _toWithdraw);
  }
  uint256 _after = IERC20(_output).balanceOf(address(this));
  uint256 _diff = _after.sub(_balance);
  if (_diff < _toWithdraw) {
      _amount = _balance.add(_diff);
  }
}

we note that this logic should only be applied under normal circumstances, where the _amount is assumed to be in the token decimals.

Recommended Mitigation Steps

_amount needs to be un-normalised back to its native token decimals.

[GalloDaSballo]

Duplicate of #131