Possible griefing attack on Router

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xd0d4dfb88161e11f6a1660b3cd7229fdf721ad2b0d0999e4b915c6490f5c24dd Severity: medium

Description: In the Router contract, the splitPosition and mergePositions functions allow users to specify an amount of tokens to be split or merged. However, the contract does not validate the amount parameter, which means that if an attacker calls these functions with 0 as the amount, it could lead to unintended behavior. Specifically, this could allow the attacker to bypass the expected token transfer and splitting logic, potentially resulting in erroneous state changes or unexpected outcomes in the contract.

Attack Scenario\

• Calling splitPosition with 0: An attacker can call the splitPosition function, passing 0 as the amount. Since the function transfers the specified amount of collateral tokens to the Router contract, this would effectively do nothing, as transferring 0 tokens is a no-op. However, this could allow the attacker to circumvent any checks or subsequent logic that relies on a non-zero amount, possibly impacting contract state or triggering unexpected behavior in downstream calls.

• Calling mergePositions with 0: Similarly, if an attacker calls the mergePositions function with 0, the logic that merges positions and processes outcomes may not operate as intended. This could lead to situations where the contract’s internal state is not updated correctly, allowing the attacker to exploit any logical vulnerabilities or unexpected behavior.

In both cases, the lack of validation on the amount parameter allows for potentially malicious behavior without proper safeguards.

The fungible address becomes overwhelmed, processing these trivial deposits, and legitimate users face delays in having their transactions processed Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)\ Set a minimum threshold for deposits to ensure only economically meaningful transactions are processed. To mitigate this vulnerability, the splitPosition and mergePositions functions should include validation to ensure that the amount parameter is greater than zero.

    function splitPosition(IERC20 collateralToken, Market market, uint256 amount) public {
+       require(amount > 0, "Amount must be greater than zero"); // Validation added
        // Existing logic...
    }

    function mergePositions(IERC20 collateralToken, Market market, uint256 amount) public {
+       require(amount > 0, "Amount must be greater than zero"); // Validation added
        // Existing logic...
    }

[greenlucid]

For this to be a valid vulnerability you have to prove that you can break something through zero amount transactions, the burden of proof is on you

[clesaege]

Doing 0 is OK. I haven't see any issue described in this report.