Careful while using tx.origin

In solidity we have two methods to access the address of caller or transaction maker, tx.origin and msg.sender. Following are the differences between both:

• tx.origin - Alice -> contract_A -> contract_B; tx.origin = Alice
• msg.sender - Alice -> contract_A -> contract_B; msg.sender = contract_A

It's important to be careful while using tx.origin, as this opens possibilities of phishing attacks.

Let's understand this with an example:

• Alice creates a wallet contract (Victim) that accepts ETH from anyone and only lets Alice transfer/withdraw.
• in the transfer() function, we can see tx.origin is used to verify if caller was Alice.

contract Victim {
    address owner;

    constructor() {
        owner = msg.sender; // Alice
    }

    receive() external payable {

    }

    function transfer(address _to, uint amount) external payable {
        require(tx.origin == owner, "Only owner can cause this txn");

        (bool sent,) = _to.call{value: amount}("");
        require(sent, "Send txn failed");
    }

    function getBalance() external view returns(uint) {
        return address(this).balance;
    }
}

The attacker can take advantage of this by scamming Alice to call attack() function of the Attacker contract.

• This will call transfer() function of the Victim contract where tx.origin will be equal to Alice.

interface IVictim{
    function transfer(address _to, uint amount) external payable;
    function getBalance() external view returns(uint);
}

contract Attacker {
    address attacker;
    IVictim victimContract;

    constructor(address _victim) {
        attacker = msg.sender;
        victimContract = IVictim(_victim);
    }

    function attack() external payable { // attacker will trick Alice to call this function
        victimContract.transfer(attacker, address(victimContract).balance);
    }
}

Such phishing attacks can be prevented by using msg.sender in place of tx.origin.
Because even if Alice is scammed to call attack() function of the Attacker contract, when this will call transfer() function, msg.sender will be equal to the address of Attacker contract. Hence transaction will fail.

contract VictimSafe {
    address owner;

    constructor() {
        owner = msg.sender;
    }

    receive() external payable {

    }

    function transfer(address _to, uint amount) external payable {
        require(msg.sender == owner, "Only owner can cause this txn");
        // changing tx.origin to msg.sender, to avoid phishing attack

        (bool sent,) = _to.call{value: amount}("");
        require(sent, "Send txn failed");
    }

    function getBalance() external view returns(uint) {
        return address(this).balance;
    }
}