Any allowance allows unlimited withdrawal changes
Description
OstiumVault is an ERC-4626 vault, which means its shares are ERC-20 tokens. One feature set by the ERC-20 standard is that an address can set an allowance amount for another address, allowing the other address to spend a limited or unlimited amount of tokens on its behalf.
The functions makeWithdrawalRequest
and cancelWithdrawalRequest
check the caller's allowance on the owner and revert if the amount of shares requested exceeds the caller's allowance:
function makeWithdrawRequest(uint256 shares, address owner) external {
// [...]
address sender = _msgSender();
uint256 allowance = allowance(owner, sender);
if (sender != owner && allowance < shares) {
revert NotAllowed(sender);
}
// [...]
}
function cancelWithdrawRequest(uint256 shares, address owner, uint16 unlockEpoch) external {
// [...]
address sender = _msgSender();
uint256 allowance = allowance(owner, sender);
if (sender != owner && allowance < shares) {
revert NotAllowed(sender);
}
// [...]
}
However, unlike transfer
and transferFrom
, these functions do not change the quantity of allowance when they succeed. As a result, there is nothing preventing the sender from making more withdrawal requests or canceling requests in excess of their allowance.
Impact
Gas notwithstanding, if an owner allows another user to spend any amount of their vault shares, then that other user can request the withdrawal of an unlimited amount of shares and also cancel the withdrawal of an unlimited amount of shares, regardless of if they initiated the withdrawal.
Recommendations
There is no way to use the single allowance quantity to track withdrawal allowance, withdrawal cancellation allowance, and ERC-20 transfer allowance. We recommend selecting one of the following options:
Explicitly allow anyone with any nonzero allowance to withdraw/cancel any amount of funds. If this option is selected, we recommend removing the misleading limit from the code.
Only allow users with unlimited allowance to withdraw/cancel any amount of funds, to reflect the unlimited nature of the permission.
Disallow anyone from making or canceling withdrawal requests on behalf of anyone else, and require the owner issue a withdrawal request if a third party is to be able to withdraw later on their behalf.
Implement a more detailed allowance system, with a separate allowance that is spent for withdrawal requests. Cancellation allowances will need to be considered carefully in this case.
Remediation
This issue has been acknowledged by Ostium Labs, and a fix was implemented in commit 79894c59↗.