040.md

Glamorous Tweed Albatross

High

Unchecked External Call in execute Function

Summary

The use of an unchecked external call in the execute function allows arbitrary interactions with external contracts. This introduces the risk of reentrancy attacks, malicious code execution, and other unexpected behaviors. The contract does not validate the target or restrict the data passed in txData, potentially leading to severe security vulnerabilities.

Root Cause

The use of target.call(txData) without validation or restrictions exposes the contract to external manipulation.

1- Arbitrary External Call: The execute function does not restrict the target address, allowing interaction with any contract. 2= Lack of Validation: txData is passed as-is to the external call, without verifying its contents or purpose. 3- Reentrancy and Exploits: Malicious contracts can exploit the unchecked call to re-enter the contract, manipulate its state, or execute harmful operations.

The following code from the execute function demonstrates the unchecked external call: https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L542-L547

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Attack Scenario

Setup: Attacker deploys a malicious contract that overrides its fallback function to execute reentrant calls or manipulate state.

Exploit:

• Attacker submits a transaction calling the execute function, providing their malicious contract as the target and crafted txData.
• The malicious contract receives the external call and triggers its fallback function.

During execution:

• The malicious contract re-enters the vulnerable contract.
• It manipulates the state (e.g., draining funds, bypassing validations, or disrupting operations).

PoC Contract

// Malicious contract
contract Malicious {
    VulnerableContract public vulnerable;

    constructor(address _vulnerable) {
        vulnerable = VulnerableContract(_vulnerable);
    }

    fallback() external payable {
        // Reenter the vulnerable contract
        vulnerable.execute(address(this), abi.encodeWithSignature("attack()"), 1 ether, IERC20(address(0)), IERC20(address(0)), 100);
    }
}

Impact

Reentrancy Attacks: An attacker can create a malicious contract that exploits reentrancy to drain funds or disrupt state transitions during the execute call.

Execution of Malicious Code: If target is an untrusted or rogue contract, harmful operations could be executed.

Denial of Service: Errors or infinite loops within the called contract could lock up the calling contract, preventing further operations.

Loss of Funds: Funds may be transferred or manipulated by malicious entities during the external call.

PoC

// Malicious contract
contract Malicious {
    VulnerableContract public vulnerable;

    constructor(address _vulnerable) {
        vulnerable = VulnerableContract(_vulnerable);
    }

    fallback() external payable {
        // Reenter the vulnerable contract
        vulnerable.execute(address(this), abi.encodeWithSignature("attack()"), 1 ether, IERC20(address(0)), IERC20(address(0)), 100);
    }
}

Mitigation

Restrict External Calls: Maintain a whitelist of approved target addresses. Only allow interactions with trusted contracts.

mapping(address => bool) public approvedTargets;

function approveTarget(address target) external onlyOwner {
    approvedTargets[target] = true;
}

require(approvedTargets[target], "Target not approved");

Use Function Interfaces: Replace raw calls with function calls through defined interfaces for specific operations.

ITrustedContract(target).execute(txData);

Validate txData: Ensure the provided data aligns with the expected input format and logic.

require(validateTxData(txData), "Invalid txData");

Fixed Code

function execute(
    address target,
    bytes memory txData,
    uint256 amountIn,
    IERC20 tokenIn,
    IERC20 tokenOut,
    uint16 bips
) internal nonReentrant returns (uint256 swapAmountOut, uint256 tokenInRefund) {
    // Validate target
    require(approvedTargets[target], "Target not approved");

    // Validate txData
    require(txData.length > 0, "Invalid txData");

    // Approve token transfer
    tokenIn.safeApprove(target, amountIn);

    // Perform the external call
    (bool success, bytes memory result) = target.call(txData);
    require(success, "External call failed");

    // Process the result...
}