`msg.sender` instead of `tx.origin`

Question

I am new to solidity but I have read at many times that tx.origin should be avoided & msg.sender should be used . There is a given demo on this in solidity page. It says like :-

Never use tx.origin for authorization. Let’s say you have a wallet contract like this:

// SPDX-License-Identifier: GPL-3.0
pragma solidity >=0.7.0 <0.9.0;
// THIS CONTRACT CONTAINS A BUG - DO NOT USE
contract TxUserWallet {
    address owner;

    constructor() {
        owner = msg.sender;
    }

    function transferTo(address payable dest, uint amount) public {
        // THE BUG IS RIGHT HERE, you must use msg.sender instead of tx.origin
        require(tx.origin == owner);
        dest.transfer(amount);    // .transfer is a global variable
    }
}

Now someone tricks you into sending Ether to the address of this attack wallet:

// SPDX-License-Identifier: GPL-3.0
pragma solidity >=0.7.0 <0.9.0;
interface TxUserWallet {
    function transferTo(address payable dest, uint amount) external;
}

contract TxAttackWallet {
    address payable owner;

    constructor() {
        owner = payable(msg.sender);
    }

    receive() external payable {
        TxUserWallet(msg.sender).transferTo(owner, msg.sender.balance);   //  **LINE 1**
    }
}

Now , I want to know that how line 1 will drain entire funds from TxUserWallet. I think transfer() is a global variable which will just transfer the amount to address in dest which is of attack wallet. How .transfer() will trigger receive() function of contract TxAttackWallet. Secondly , In line TxUserWallet(msg.sender).transferTo(owner, msg.sender.balance); , why are we writing it as TxUserWallet(msg.sender) , like why are we adding (msg.sender) after contract name and also what values is being passed by writing msg.sender.balance ?

Answer 1

the receive function is triggered by receiving eth.

the msg.sender in that line is the address of the vulnerable wallet/contract. The wallet name is an instance or interface for that type of contract, and it needs the address to instantiate it to interact with it.

msg.sender.balance is getting the balance from the vulnerable contract so the attack contract can know how much it can transfer out of the vulnerable contract

that line calls the transferTo function of the vulnerable contract passing a to address controlled by the attacker, and amount of the whole balance of the vulnerable contract.

The wallet contract tries to limit its transferTo being used by anyone other than its owner by ensuring only its owner can call that function with tx.origin instead of msg.sender.

tx.origin will always stay the same. i.e. the EOA that initiated the chain of transactions. msg.sender doesn't stay the same and, in this case would change to the attack contract address when the attack contract calls the transferTo function.

If a owner of the vulnerable contract is tricked into initiating a transaction on the attack contract, the attack contract can then call the transferTo function on the vulnerable contract and the vulnerable contract's check will still conclude it's coming from its owner as it uses tx.origin as a check. It will allow the transfer.

If it checked using msg.sender, it would "see" that the function wasn't being called directly by the owner, but by a contract and so not allow the transfer as the attack contract address is not the vulnerable contract's owner

For the attack scenario to be demoed the vulnerable contract needs a fallback/receive function so it can be funded and have a balance. Yes, it could be done with a forced send instead, but that would be silly.