Category: Business Logic
Incorrect calculation in refundGas
Medium Severity
Medium Impact
High Likelihood
Description
The Vault::refundGas
function performs an incorrect calculation of the amountRefundable
variable if the WETH amount to unwrap is greater than the available balance. The code is reported below for convenience:
function refundGas(uint256 _amount, address _dst) external onlyPaymaster {
uint256 amountRefundable = _amount;
uint256 ethBal = address(this).balance;
// we need to unwrap some WETH in this case.
if (ethBal < _amount) {
IWETH9 weth9 = IWETH9(LibVaultStorage.ds().weth);
uint256 balanceWETH9 = weth9.balanceOf(address(this));
uint256 toUnwrap = _amount - ethBal;
// this should not happen, but if it does, we should take what we can instead of reverting
if (toUnwrap > balanceWETH9) {
weth9.withdraw(balanceWETH9);
amountRefundable = amountRefundable - toUnwrap - balanceWETH9;
} else {
weth9.withdraw(toUnwrap);
}
}
// [code continues...]
Consider the following numerical example:
_amount
is 100ethBal
is 60balanceWETH9
is 30toUnwrap
will be calculated as 100 - 60 = 40amountRefundable
will be calculated as 100 - 40 - 30 = 30, instead of the expected 90
Impact
The function will refund to the treasury less than the expected amount.
Recommendations
Fix the calculation by applying parentheses around toUnwrap - balanceWETH9
on the line calculating amountRefundable
.
Remediation
Voyage has followed the recommendation and corrected the calculation in commit 6e44df5f↗.