Glamorous Tweed Albatross
High
The execute function introduces a race condition vulnerability by using safeApprove without resetting the existing token allowance to zero. This allows an attacker to front-run the transaction, exploit the existing allowance, and execute unauthorized token transfers. This vulnerability can result in financial losses and contract misbehavior, undermining the integrity of the system.
File: Bracket.sol https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L526-L542
Issue Flow Approval Without Reset:
- The function sets an allowance for target without resetting the previous allowance to zero.
- Existing token allowance is not invalidated, creating an opportunity for exploitation.
Front-Running Opportunity:
- An attacker observes the transaction in the mempool and front-runs it by using the existing allowance for malicious token transfers.
Execution of Original Transaction:
- The original transaction continues after the attacker's front-running transaction, potentially compounding the financial loss.
No response
No response
Setup: 1- Alice uses the contract to execute a swap for 1,000 tokenIn. 2- The contract sets an allowance of 1,000 tokenIn for the swap target (target). 3- The previous transaction had left an allowance of 500 tokenIn for the same target. Attack: 1- Bob, the attacker, monitors the mempool and observes Alice’s transaction. 2- Before Alice’s transaction is mined, Bob submits a front-running transaction using the existing 500 tokenIn allowance to transfer tokens to himself.
Outcome:
- Bob successfully transfers 500 tokenIn to himself.
- Alice’s transaction executes as intended but with the remaining tokens, causing unintended token movements.
Financial Loss: Attackers can steal tokens from the contract by exploiting the existing allowance.
System Integrity: Users lose trust in the platform if unauthorized transactions occur.
Scalability Risks: Large-scale exploitation of this vulnerability can cause significant damage to the protocol and its reputation.
No response
Modify the execute function to reset the allowance to zero before setting a new allowance. This ensures the old allowance cannot be exploited.
Fixed Code:
function execute(
address target,
bytes memory txData,
uint256 amountIn,
IERC20 tokenIn,
IERC20 tokenOut,
uint16 bips
) internal nonReentrant returns (uint256 swapAmountOut, uint256 tokenInRefund) {
// Reset the allowance to zero to prevent race conditions
tokenIn.safeApprove(target, 0);
// Set the required allowance
tokenIn.safeApprove(target, amountIn);
// Perform the external call
(bool success, bytes memory result) = target.call(txData);
require(success, "External call failed");
// Process results
uint256 finalTokenIn = tokenIn.balanceOf(address(this));
uint256 finalTokenOut = tokenOut.balanceOf(address(this));
require(finalTokenOut > 0, "No tokens received");
swapAmountOut = finalTokenOut - amountIn;
tokenInRefund = amountIn - (finalTokenIn - amountIn);
}