Liquidity Management

in the GmxProxy contract

Summary:

The function setPerpVault(...) uses tx.origin == owner() instead of the more standard msg.sender == owner(). This reliance on tx.origin poses a security risk and deviates from the contract’s typical ownership pattern.

Because setPerpVault uses tx.origin == owner() instead of the typical msg.sender == owner(), it’s vulnerable to indirect calls through malicious contracts that can hijack or redirect perpVault assignment. Standardizing on the usual “onlyOwner” pattern is safer and consistent with the rest of the contract.

RootCause & Where It Happens

Inside the setPerpVault function:

Notice that to restrict calls to the owner, the code checks:

while elsewhere in the contract, or in typical OpenZeppelin patterns, you use something like:

Why tx.origin Is Problematic

1. Susceptible to Proxy/Phishing Attacks

   • A malicious contract can trick an EOA user (who is the owner) into calling it. Because a user’s transaction going through another contract still has the same tx.origin (the user’s address), your setPerpVault call might be invoked under unexpected circumstances.

   • For instance, the malicious contract can call your function with the user’s tx.origin while msg.sender is the attacker contract. The check tx.origin == owner() passes, even though the direct caller is not owner.

2. Breaks the Usual “OnlyOwner” Pattern

   • Everywhere else, the code either uses onlyOwner or checks msg.sender == owner(), ensuring direct calls by the owner. Here, the logic is inconsistent. If later you rely on the same assumption that “the function’s msg.sender must be owner,” you might incorrectly trust calls from another contract.

3. Deviates from the rest of the contract

   • The rest of GmxProxy (and typical upgradeable OpenZeppelin code) uses onlyOwner or checks msg.sender == owner(). The mismatch leads to confusion or an unexpected security gap specifically in setPerpVault().

Impact

• Potential Unauthorized Setting of perpVault: A malicious contract could engineer a call from the real owner’s EOA (with tx.origin == owner()) but route it through malicious code, letting that malicious contract control which _perpVault is set.

• Once perpVault is set to a malicious address, the GmxProxy contract will forward funds, create orders, or do other logic tied to this malicious vault address—severely compromising the system.

Foundry PoC

Recommended Remediation

1. Use msg.sender == owner() (like standard “onlyOwner” modifiers) instead of tx.origin. That is the canonical, secure way to restrict calls to the owner.

2. If you truly need the EOA check (very rare), adopt a well-documented flow that reverts if the call is from a contract, but this is generally discouraged in DeFi for flexibility and composability.

   // Typically not recommended, but if absolutely needed:

   require(msg.sender == tx.origin && msg.sender == owner(), "Must be EOA and owner");

   ​

   Even then, there are known techniques to circumvent EOA checks in advanced contexts.