Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Unsafe Use of tx.origin in setPerpVault Function

Deleted user

Summary

The setPerpVault function in GmxProxy.sol uses tx.origin for access control, which is a dangerous practice that could allow phishing attacks to bypass owner verification.

Vulnerability Details

function setPerpVault(address _perpVault, address market) external {
require(tx.origin == owner(), "not owner"); // Vulnerable line
require(_perpVault != address(0), "zero address");
require(perpVault == address(0), "already set");
perpVault = _perpVault;
gExchangeRouter.setSavedCallbackContract(market, address(this));
}

The vulnerability exists because:

  1. tx.origin refers to the original externally owned account (EOA) that initiated the transaction

  2. Using tx.origin for authentication makes the contract vulnerable to phishing attacks

  3. An attacker can trick the owner into interacting with a malicious contract that calls setPerpVault

Impact

Severity: Medium

  • Likelihood: Medium (requires social engineering)

  • Impact: Medium (only affects initial setup)

Attack Scenario:

  1. Attacker deploys malicious contract

  2. Owner interacts with malicious contract

  3. Malicious contract calls setPerpVault with attacker's parameters

  4. Check passes because tx.origin is owner's address

  5. Attacker gains control of vault configuration

Tools Used

  • Manual code review

  • Solidity visual developer

Recommendations

Replace tx.origin with msg.sender and use OpenZeppelin's onlyOwner modifier:

function setPerpVault(address _perpVault, address market) external onlyOwner {
require(_perpVault != address(0), "zero address");
require(perpVault == address(0), "already set");
// Add event emission for better tracking
emit PerpVaultSet(_perpVault, market);
perpVault = _perpVault;
gExchangeRouter.setSavedCallbackContract(market, address(this));
}
​
// Add event definition
event PerpVaultSet(address indexed perpVault, address indexed market);

Additional recommendations:

  1. Add timelock for critical parameter changes

  2. Implement two-step transfer pattern for ownership changes

  3. Add emergency pause mechanism

  4. Consider multi-signature requirements for critical functions

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

invalid_tx-origin

Lightchaser: Medium-5

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!