Jumpy Sage Pony - An attacker can take advantage of the StopLimit's unlimited allowance to Bracket to steal funds from the StopLimit.

[sherlock-admin2]

Jumpy Sage Pony

High

An attacker can take advantage of the StopLimit's unlimited allowance to Bracket to steal funds from the StopLimit.

Summary

The StopLimit's allowance to Bracket is set to be unlimited. An attacker can exploit this unlimited allowance to steal all funds stored in the StopLimit.

Root Cause

The StopLimit's allowance to Bracket is set to be unlimited.
https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/StopLimit.sol#L99-L104

        //approve
@>      updateApproval(
@>          address(BRACKET_CONTRACT),
            order.tokenIn,
            order.amountIn
        );

https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/StopLimit.sol#L397-L411

        ///@notice if current approval is insufficient, approve max
        ///@notice oz safeIncreaseAllowance controls for tokens that require allowance to be reset to 0 before increasing again
        function updateApproval(
                address spender,
                IERC20 token,
                uint256 amount
        ) internal {
                // get current allowance
                uint256 currentAllowance = token.allowance(address(this), spender);
                if (currentAllowance < amount) {
                // amount is a delta, so need to pass max - current to avoid overflow
@>              token.safeIncreaseAllowance(
@>                      spender,
@>                      type(uint256).max - currentAllowance
                );
                }
        }

An attacker can create an order with minimum amountIn in the Bracket contract and calls performUpkeep with target as the tokenToSteal and txData as transferFrom(stopLimit, this, amountToSteal).
Then all tokenToSteal will be transferred to Bracket from StopLimit and finally be given to the attacker.
As a result, the attacker steal funds from the StopLimit.

https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/StopLimit.sol#L526-L568

    function execute(
        address target,
        bytes memory txData,
        uint256 amountIn,
        IERC20 tokenIn,
        IERC20 tokenOut,
        uint16 bips
    ) internal returns (uint256 swapAmountOut, uint256 tokenInRefund) {
        //update accounting
        uint256 initialTokenIn = tokenIn.balanceOf(address(this));
        uint256 initialTokenOut = tokenOut.balanceOf(address(this));

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

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

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

            //if success, we expect tokenIn balance to decrease by amountIn
            //and tokenOut balance to increase by at least minAmountReceived
            require(
                finalTokenOut - initialTokenOut >
                    MASTER.getMinAmountReceived(
                        amountIn,
                        tokenIn,
                        tokenOut,
                        bips
                    ),
                "Too Little Received"
            );

562:        swapAmountOut = finalTokenOut - initialTokenOut;
            tokenInRefund = amountIn - (initialTokenIn - finalTokenIn);
        } else {
            //force revert
            revert TransactionFailed(result);
        }
    }

Internal pre-conditions

none

External pre-conditions

none

Attack Path

1. Alice creates an order with tokenOut as the token to steal from the StopLimit.
2. Alice calls performUpkeep().(This can be done just after creating order, it the TP or SL is set as the current price).
   - target : tokenToSteal
   - txData : transferFrom(stopLimit, this, amountToSteal)

Then all tokenToSteal will be transferred to Bracket from StopLimit and finally be given to the attacker at L562.
As a result, the attacker steal funds from the StopLimit.

Impact

An attacker can steal the funds from StopLimit.

PoC

none

Mitigation

The allowance to the Bracket should not be set type(uint256).max.

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits:
gfx-labs/oku-custom-order-types#1

[sherlock-admin3]

The protocol team fixed this issue in the following PRs/commits:
gfx-labs/oku-custom-order-types#1