Skip to content

Latest commit

 

History

History
35 lines (25 loc) · 2.19 KB

071.md

File metadata and controls

35 lines (25 loc) · 2.19 KB

Bald Honeysuckle Urchin

Medium

SafeERC20.safeApprove reverts for changing existing approvals

Summary

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.

Root Cause

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.

Impact

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

Mitigation

Consider using safeIncreaseAllowance and safeDecreaseAllowance instead of safeApprove in Bracket::execute() and OracleLess::execute()