Bald Honeysuckle Urchin
Medium
SafeERC20.safeApprove
reverts when a non-zero approval is changed to a non-zero approval. In Bracket::execute()
in line 539,safeApprove
is used to change approval. This will revert if approval is non zero causing the target swap contract unusable. It is also used in OracleLess::execute
to approve a target contract for swap. According to OpenZeppelin’s documentation, safeApprove
is a deprecated method with known limitations and potential vulnerabilities, such as resetting a non-zero allowance to another non-zero value. The function should instead use safeIncreaseAllowance
or safeDecreaseAllowance
to manage token approvals securely.
The safeApprove()
function has explicit warning:
/**
* @dev Deprecated. This function has issues similar to the ones found in
* {IERC20-approve}, and its usage is discouraged.
*
* Whenever possible, use {safeIncreaseAllowance} and
* {safeDecreaseAllowance} instead.
*/
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'
The function will always revert if existing approval is not zero.
This approval is meant to approve a contract which will swap the tokens. If approval is non zero, safeApprove
will always revert making the swap operation on the target contract unusable. Possible DoS since performUpkeep
calls execute()
using the same target contract all the time for swap operation in Bracket::execute()
. In case of OracleLess
, the target contract will be unusable by the OracleLess
contract, since it will always revert while trying to set allowance
Consider using safeIncreaseAllowance
and safeDecreaseAllowance
instead of safeApprove
in Bracket::execute()
and OracleLess::execute()