Skip to content

Latest commit

 

History

History
100 lines (77 loc) · 4.33 KB

010.md

File metadata and controls

100 lines (77 loc) · 4.33 KB

Calm Sky Hippo

High

Generating duplicate order IDs in generateOrderId function causing transaction conflicts

Summary

The generateOrderId function in the contract relies on the block.timestamp and the sender’s address to create unique order IDs. However, this approach is susceptible to generating the same order ID in cases of transactions occurring in the same block and with the same sender. This can lead to issues with order identification, potentially causing transaction conflicts and inconsistencies.

Root Cause

The generateOrderId function uses block.timestamp and the sender’s address to generate an order ID, using the following line of code: https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/AutomationMaster.sol#L90-L95

uint256 hashedValue = uint256(
    keccak256(abi.encodePacked(sender, block.timestamp))
);

While this approach can be effective in most scenarios, it has a vulnerability. Block timestamps are not guaranteed to be unique, as multiple transactions can be mined in the same block with the same timestamp. Since the timestamp is used directly to calculate the order ID, it's possible for two different transactions with the same sender to generate the same order ID, especially if they occur in the same block.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

  1. Assumptions: The same address (sender) invokes the generateOrderId function in two separate transactions. Both transactions are mined in the same block, meaning the block.timestamp will be the same for both.
  2. Example inputs: Sender address: 0x1234567890abcdef1234567890abcdef12345678 Block timestamp: 1680000000
  3. Execution: First transaction:
address sender = 0x1234567890abcdef1234567890abcdef12345678;
uint256 hashedValue = uint256(keccak256(abi.encodePacked(sender, 1680000000)));
uint96 orderId1 = uint96(hashedValue);

Second transaction (same sender, same block timestamp):

address sender = 0x1234567890abcdef1234567890abcdef12345678;
uint256 hashedValue = uint256(keccak256(abi.encodePacked(sender, 1680000000)));
uint96 orderId2 = uint96(hashedValue);

Since both transactions share the same sender and block.timestamp, the keccak256 hash will be identical in both cases, and thus the resulting order IDs (orderId1 and orderId2) will be the same.

Impact

  1. If two orders with the same ID are created, the system could mistakenly treat them as the same order, leading to conflicts when trying to execute or track orders.
  2. The order ID should be unique to ensure that each order can be tracked independently. Duplicate IDs can cause severe issues in automated trading or swap systems, where unique identification of each order is crucial for proper execution.
  3. Multiple transactions occurring at the same time could conflict, leading to incorrect or failed executions.

PoC

// Hardhat test to demonstrate vulnerability
const { expect } = require("chai");

describe("Order ID Generation", function () {
  let contract;

  beforeEach(async () => {
    const [owner] = await ethers.getSigners();
    const ContractFactory = await ethers.getContractFactory("YourContract");
    contract = await ContractFactory.deploy();
  });

  it("should generate the same order ID for the same sender in the same block", async function () {
    const sender = "0x1234567890abcdef1234567890abcdef12345678";

    const orderId1 = await contract.generateOrderId(sender);
    const orderId2 = await contract.generateOrderId(sender);

    expect(orderId1).to.equal(orderId2);  // The order IDs should be the same
  });
});

Output:

  Order ID Generation
    ✓ should generate the same order ID for the same sender in the same block

Mitigation

To mitigate this issue, I recommend adding more unique factors to the order ID generation, such as the block number or transaction nonce (which is unique per sender per transaction). This will reduce the likelihood of collisions between two transactions from the same sender in the same block.

function generateOrderId(address sender) external view override returns (uint96) {
    uint256 hashedValue = uint256(
        keccak256(abi.encodePacked(sender, block.timestamp, block.number, msg.sender))
    );
    return uint96(hashedValue);
}