code-423n4 / 2022-01-insure-findings

2 stars 0 forks source link

[WP-L28] `Vault#_unutilize()` Lack of validation for the amount of funds received #270

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2022-01-insure/blob/19d1a7819fe7ce795e6d4814e7ddf8b8e1323df3/contracts/Vault.sol#L429-L434

function _unutilize(uint256 _amount) internal {
    require(address(controller) != address(0), "ERROR_CONTROLLER_NOT_SET");

    controller.withdraw(address(this), _amount);
    balance += _amount;
}

Recommendation

Can be changed to:

function _unutilize(uint256 _amount) internal {
    require(address(controller) != address(0), "ERROR_CONTROLLER_NOT_SET");

    uint256 beforeBalance = IERC20(token).balanceOf(address(this));
    controller.withdraw(address(this), _amount);
    uint256 received = IERC20(token).balanceOf(address(this)) - beforeBalance;
    require(received >= _amount, "...");
    balance += received;
}
0xHaku commented 2 years ago

@oishun1112 what error message do we set at require(received >= _amount, "...");?

oishun1112 commented 2 years ago

@taka0409 let's have "ERROR_INSUFFICIENT_RETURN_VALUE"