Lively Corduroy Crab - Unspent token allowance blocks reuse of swap targets

[sherlock-admin3]

Lively Corduroy Crab

Medium

Unspent token allowance blocks reuse of swap targets

Summary

The use of safeApprove in the execute function will cause a revert for the protocol when attempting token swaps via the target contract if a non-zero allowance already exists. This occurs because safeApprove reverts when trying to set an allowance on a token that already has a non-zero approval amount.

function execute(
        address target,
        bytes calldata txData,
        Order memory order
    ) internal returns (uint256 amountOut, uint256 tokenInRefund) {
        //update accounting
        uint256 initialTokenIn = order.tokenIn.balanceOf(address(this));
        uint256 initialTokenOut = order.tokenOut.balanceOf(address(this));

        //approve
        order.tokenIn.safeApprove(target, order.amountIn);

        //perform the call
        (bool success, bytes memory reason) = target.call(txData);

        if (!success) {
            revert TransactionFailed(reason);
        }

        uint256 finalTokenIn = order.tokenIn.balanceOf(address(this));
        require(finalTokenIn >= initialTokenIn - order.amountIn, "over spend");
        uint256 finalTokenOut = order.tokenOut.balanceOf(address(this));

        require(
            finalTokenOut - initialTokenOut > order.minAmountOut,
            "Too Little Received"
        );

        amountOut = finalTokenOut - initialTokenOut;
        tokenInRefund = order.amountIn - (initialTokenIn - finalTokenIn);
    }

Root Cause

In the execute function of OracleLess.sol, the following code creates the root cause:
https://github.com/sherlock-audit/2024-11-oku/blob/ee3f781a73d65e33fb452c9a44eb1337c5cfdbd6/oku-custom-order-types/contracts/automatedTrigger/OracleLess.sol#L237

order.tokenIn.safeApprove(target, order.amountIn);

The safeApprove function from the OpenZeppelin library requires that the allowance for target on order.tokenIn must first be set to zero before being re-approved. However, in certain scenarios, leftover approvals from partial token usage during previous swaps (e.g., unspent tokenIn is refunded but allowance remains) will result in the next call to safeApprove reverting.

//refund any unspent tokenIn
        //this should generally be 0 when using exact input for swaps, which is recommended

Any malicious user could perform fillOrder without fully utilizing the amount, managing to DoS that target, preventing it from being used for that token.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. A malicious user initiates a fillOrder to swap the tokens, which triggers the execute function. During this process, the function sets the token allowances for the target address.
2. The swap executes but does not utilize the entire allowance.
3. Next user attempts to fill an order using the same swap target.
4. Since the leftover allowance is non-zero, the safeApprove function call in the execute function will revert.

This vulnerability prevents the protocol from using the same target for any future swaps involving the same token.

Impact

A malicious user (or even a non-malicious user who is not aware of the issue) could execute a swap without fully utilizing the approved amount of tokens. This leftover allowance for the target address and token pair will prevent any future fillOrder attempts using the same target and token pair from succeeding, as the safeApprove function will fail due to the pre-existing non-zero allowance. This could disrupt the expected behavior of the protocol and prevent users from executing swaps correctly. Furthermore, a malicious user could exploit this by targeting any swap target they choose, making it possible to disrupt the protocol’s operations across multiple targets and token pairs, leading to failed transactions and potential losses of funds.

PoC

No response

Mitigation

Consider updating the code to reset the allowance to zero before setting a new allowance.

+  order.tokenIn.safeApprove(target, 0);  
   order.tokenIn.safeApprove(target, order.amountIn);

This ensures compatibility with the safeApprove function's requirements and prevents issues caused by leftover allowances.