Atomic Cider Kangaroo - Denial of Service Risk due to Improper Use of safeApprove()

[sherlock-admin3]

Atomic Cider Kangaroo

Medium

Denial of Service Risk due to Improper Use of safeApprove()

Vulnerability Detail

Link
An unspent allowance may cause a denial of service during the calls to safeApprove() in the 'Bracket.sol' contract.

The contract uses the safeApprove() function to grant permission to spend funds.

The safeApprove() function is a wrapper provided by the SafeERC20 library present in the OpenZeppelin contracts package, its implementation is the following:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/utils/SafeERC20.sol#L45-L54

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));
}

As the comment warns, this should only be used when setting an initial balance or resetting it to zero. In the bracket and Oracleless contracts, the use of safeApprove() is included in the functions like execute. The execute fuction is called by _createOrderWithSwap function, which is indirectly called by the external createorder function. The target can be permanently DOSsed for lack of approval to zero
This means that any unspent allowance of tokens will cause a denial of service in the functions, potentially bricking the contract.

Recommended Mitigation

1. Reset the allowance to zero before calling safeApprove()