sherlock-audit / 2024-02-rubicon-finance-judging

5 stars 3 forks source link

bigbick123456789000 - Lack of Protection Against Replay Attacks in PartialFillLib Contract Causing Repeated Messages #13

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

bigbick123456789000

medium

Lack of Protection Against Replay Attacks in PartialFillLib Contract Causing Repeated Messages

Summary

The provided Solidity contract and library lack mechanisms to prevent repeated messages or transactions. Without such protections, the contract may be vulnerable to unintended behaviour or exploitation due to duplicate actions.

Vulnerability Detail

The PartialFillLibcontract facilitates the handling of Gladius orders, including partitioning order amounts for partial fills, validating partitions and thresholds, and hashing Gladius orders for signature verification. However, it does not include explicit logic to prevent replay attacks. Without such protection, an attacker could potentially repeat valid messages, leading to unintended consequences or duplication of actions.

// Code snippet demonstrating lack of protection against replay attacks

/// @notice hash the given order
/// @param order the order to hash
/// @return the eip-712 order hash
function hash(GladiusOrder memory order) internal pure returns (bytes32) {
    return
        keccak256(
            abi.encode(
                ORDER_TYPE_HASH,
                order.info.hash(),
                order.decayStartTime,
                order.decayEndTime,
                order.exclusiveFiller,
                order.exclusivityOverrideBps,
                order.input.token,
                order.input.startAmount,
                order.input.endAmount,
                order.outputs.hash(),
                order.fillThreshold
            )
        );
}

The hash function simply concatenates various parameters of the GladiusOrder struct and hashes them. However, it does not include any additional data such as a nonce or a unique identifier that would ensure the uniqueness of each message hash. Without a nonce or unique identifier, an attacker could capture a valid message and its corresponding hash. They could then replay this message and its hash at a later time, tricking the contract into believing it is a legitimate message. Since there's no mechanism to distinguish between different invocations of the same message, the contract would accept the replayed message as valid.

Impact

The lack of protection against replay attacks in the PartialFillLibcontract could lead to unauthorized parties repeating valid messages, resulting in potential financial losses, incorrect order execution, or disruption of the intended functionality.

Code Snippet

Link

Tool used

Manual Review

Recommendation

Include a unique nonce or timestamp in each signed message, the contract can verify the freshness and uniqueness of each message, thereby preventing replay attacks.

sherlock-admin commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

0xAadi commented:

Invalid: "Does not cause any financial losses, incorrect order execution, or disruption of the intended functionality."