Skip to content

Latest commit

 

History

History
156 lines (111 loc) · 4.14 KB

025.md

File metadata and controls

156 lines (111 loc) · 4.14 KB

Flaky Merlot Parrot

High

Insufficient Access Control Will Allow Unauthorized Order Creation, Leading to Potential Fund Locks and Denial of Service for Users

Summary

The createOrder function in Bracket.sol lacks sufficient access control, allowing any caller to create orders on behalf of any recipient. This vulnerability enables unauthorized actions, such as malicious order creation or denial-of-service attacks, that may result in locked user funds or system clutter.

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

Root Cause

1- Missing Authorization Check:

The createOrder function does not verify that msg.sender matches the specified recipient:

function createOrder(
    // Parameters...
    address recipient,
    // More parameters...
) external override nonReentrant {
    _initializeOrder(
        // Parameters...
        recipient,
        // More parameters...
    );
}

2- External Visibility:

  • The function is marked external, allowing any address to call it.

Internal pre-conditions

Arbitrary Recipient Address: The caller specifies an arbitrary recipient address during order creation.

No Access Control: The function does not restrict who can create orders for a given recipient.

External pre-conditions

Public Function Accessibility: As the function is external, any address can invoke it, including attackers.

Token Transfer Approvals: The recipient must have previously approved the transfer of tokens, enabling unauthorized order creation.

Attack Path

1- An attacker calls createOrder with the following parameters:

  • recipient = victimAddress.
  • Other parameters configured to create a valid order.

2- The createOrder function initializes an order for victimAddress:

_initializeOrder(
    // Parameters...
    recipient,
    // More parameters...
);

3- The victim’s tokens are locked into an order they did not authorize.

4- The attacker can repeat the process to spam orders for the victim, leading to:

  • Locked funds.
  • System clutter, making it difficult for the victim to manage their legitimate orders.

Impact

Unauthorized Token Locks: Users may find their tokens locked in orders they did not authorize, losing access to their funds.

Denial of Service: Attackers can flood the system with unauthorized orders, disrupting the victim's ability to manage legitimate orders.

Security Concerns: The lack of proper access control undermines user trust and exposes the platform to malicious activities.

PoC

Here’s how the issue propagates through the contract:

1- createOrder Function:

function createOrder(
    // Parameters...
    address recipient,
    // More parameters...
) external override nonReentrant {
    _initializeOrder(
        // Parameters...
        recipient,
        // More parameters...
    );
}

Issue: No check ensures that msg.sender == recipient.

2- _initializeOrder Function:

This internal function assumes the caller has appropriate permissions:

function _initializeOrder(
    // Parameters...
    address recipient,
    // More parameters...
) internal {
    // Logic to initialize order for `recipient`
}

Scenario:

1- The victim (address 0xVictim) has tokens approved for transfer by the Bracket contract. 2- The attacker (address 0xAttacker) invokes createOrder with:

  • recipient = 0xVictim.

3- The contract initializes an order for 0xVictim without their consent, locking their tokens. 4- The attacker repeats this process to spam orders for 0xVictim.

Result:

  • The victim’s tokens are locked in unauthorized orders.
  • The system is cluttered with spam orders, causing denial of service.

Mitigation

Proposed Fix

Here’s the corrected implementation of createOrder with added access control:

function createOrder(
    // Parameters...
    address recipient,
    // More parameters...
) external override nonReentrant {
    require(msg.sender == recipient, "Only recipient can create order");
    _initializeOrder(
        // Parameters...
        recipient,
        // More parameters...
    );
}