069.md

Bald Honeysuckle Urchin

High

Possible hash collision when creating order

Summary

The AutomationMaster::generateOrderId function is designed to generate a unique uint96 identifier based on the sender's address and the current block timestamp. However, its current implementation can result in hash collisions when invoked multiple times within the same block.

Root Cause

The function uses the block.timestamp value as part of the input to keccak256 in line 92. Since the block.timestamp remains constant for all transactions within the same block, multiple invocations during the same block produce identical hash values.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

In scenarios similar to centralized exchanges, a user may create multiple orders in rapid succession to balance their trading portfolio. If these orders are processed within the same block, the AutomationMaster::generateOrderId function will return the same hash due to the constant block.timestamp

Impact

When hash collisions occur:

• Subsequent orders overwrite previous ones in the same block.
• The user loses the funds associated with the overwritten orders, leading to significant financial loss.

PoC

No response

Mitigation

To prevent hash collisions and ensure unique identifiers, incorporate a nonce or unique counter for each user. This ensures that even within the same block, the hash value remains distinct for every invocation.

+  mapping(address => uint256) private nonces;

    function generateOrderId(address sender) external view override returns (uint96) {
+          uint256 nonce = nonces[sender]++;
           uint256 hashedValue = uint256(
               keccak256(abi.encodePacked(sender, block.timestamp, nonce))
           );
           return uint96(hashedValue);
    }