_swapAssetOut() of AssetLogic.sol uses safeApprove() from openzeppelin's SafeERC20.
safeApprove() will revert when value != 0 and allowance to the spender != 0 .
When the _amountIn is > 0 and token.allowance(address(this), spender) > 0, the function will revert.
Proof of Concept
Snippet from SafeERC20
function safeApprove(
IERC20 token,
address spender,
uint256 value
) internal {
// safeApprove should only be called when setting an initial allowance,
// or when resetting it to zero. To increase and decrease it, use
// 'safeIncreaseAllowance' and 'safeDecreaseAllowance'
require(
(value == 0) || (token.allowance(address(this), spender) == 0),
"SafeERC20: approve from non-zero to non-zero allowance"
);
_callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, value));
}
Snippet from _swapAssetOut()
function _swapAssetOut(
// .........
// perform the swap
SafeERC20.safeApprove(IERC20(_assetIn), address(pool), _amountIn);
amountIn = pool.swapExactOut(_amountOut, _assetIn, _assetOut, _maxIn);
}
// slippage is too high to perform swap: success = false, amountIn = 0
......
}
Tools Used
Manual Analysis
Recommended Mitigation Steps
Use safeIncreaseAllowance and safeDecreaseAllowance instead of safeApprove().
Lines of code
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/libraries/AssetLogic.sol#L347
Vulnerability details
Impact
_swapAssetOut()
ofAssetLogic.sol
usessafeApprove()
from openzeppelin'sSafeERC20
.safeApprove()
will revert whenvalue != 0
and allowance to the spender != 0 ._amountIn
is > 0 andtoken.allowance(address(this), spender)
> 0, the function will revert.Proof of Concept
Snippet from SafeERC20
Snippet from
_swapAssetOut()
Tools Used
Manual Analysis
Recommended Mitigation Steps
Use
safeIncreaseAllowance
andsafeDecreaseAllowance
instead ofsafeApprove()
.