Handle

gpersoon

Vulnerability details

Impact

The function withdraw of Vault.sol calculates _toWithdraw, _after and _diff if _diff < _toWithdraw then: _amount = _balance + _diff, which is the same as: _amount = _balance + _after - _balance, which is the same as: _amount = _after

Changing this makes the code easier to read and save a bit of gas.

Proof of Concept

https://github.com/code-423n4/2021-09-yaxis/blob/main/contracts/v3/Vault.sol#L226

function withdraw(uint256 _shares, address _output) public override checkToken(_output) {
        uint256 _amount = (balance().mul(_shares)).div(totalSupply());
        ...
        uint256 _balance = IERC20(_output).balanceOf(address(this));
        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);   // == _balance + (_after - _balance) == _after 
            }
        }

Tools Used

Recommended Mitigation Steps

replace _amount = _balance.add(_diff);
with _amount = _after;

code-423n4 / 2021-09-yaxis-findings