yAxis contest - gpersoon's results

Handle

gpersoon

Vulnerability details

Impact

The function depositMultipleVault of VaultHelper doesn't check the array size of _tokens and _amounts are the same length. In previous version of solidity there were bugs with giving an enormous large array to a function which accepted memory arrays. Although depositMultipleVault uses calldata arrays, it is probably better to add a check on the length.

On the other hand the function depositMultiple of Vault.sol does check it.

Proof of Concept

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

 function depositMultipleVault(
        address _vault,
        address[] calldata _tokens,
        uint256[] calldata _amounts
    )  external {
        for (uint8 i = 0; i < _amounts.length; i++) {
        ...

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

 function depositMultiple(
        address[] calldata _tokens,
        uint256[] calldata _amounts
    )  external override notHalted returns (uint256 _shares) {
        require(_tokens.length == _amounts.length, "!length");

        for (uint8 i; i < _amounts.length; i++) {
           ... 

Tools Used

Recommended Mitigation Steps

Add something like the following in depositMultipleVault: require(_tokens.length == _amounts.length, "!length");

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;