Remove tx.origin

[PeterBorah]

Summary

tx.origin is a security vulnerability, breaks compatibility with other contracts including security contracts, and is almost never useful. Removing it would make Solidity more user-friendly. If there are exceptional cases where access to the transaction origin is needed, a library using in-line assembly can provide it.

The Problems

1. tx.origin is a security vulnerability. As we recently saw with the Mist wallet, using tx.origin makes you vulnerable to attacks comparable to phishing or cross-site scripting. Once a user has interacted with a malicious contract, that contract can then impersonate the user to any contract relying on tx.origin.

2. tx.origin breaks compatibility. Using tx.origin means that your contract cannot be used by another contract, because a contract can never be the tx.origin. This breaks the general composability of Ethereum contracts, and makes them less useful. In addition, this is another security vulnerability, because it makes security-based contracts like multisig wallets incompatible with your contract.

3. tx.origin is almost never useful. This is the most subjective point, but I have yet to come across a use of tx.origin that seemed legitimate to me. I welcome counter-examples, but I've written dozens or hundreds of smart contracts without needing it, and I have never heard of anyone else needing it either.

Rationale for Removal

Solidity's design philosophy is to prioritize security and reliability over expressiveness. In other cases where behavior is unreliable, Solidity does not expose it. (For instance, there is no way to call an external contract and retrieve the return value if the signature is not known ahead of time.)

The "escape clause" is in-line assembly, which allows the creation of libraries to do anything expressible as EVM assembly. Behavior that is unsafe and unreliable is best kept in libraries, rather than given to all users as part of the core language.

[aakilfernandes]

SafeMarket currently has a vulnerability due to use of tx.origin. Thankfully, Peter explained this bug to me otherwise I would have fallen into the trap.

[chriseth]

I'm not sure removing is a good idea, but we can add a warning.

[PeterBorah]

Why not?

[chriseth]

Because it will break backwards-compatibility. If you are saying that a warning is not enough and will not be noticed, we should revisit how people compile their contracts.

[chriseth]

If someone wants to implement that, please take a look at #677 which is very similar. This one has to go into visit(MemberAccess ...).

[PeterBorah]

Fair enough. Is Solidity committed to backwards-compatibility in general?

I would recommend at least adding it to the list of things to change when you do the next breaking change. Any code that would be broken by this needs to be broken, because it is almost certainly insecure in some way.

[redsquirrel]

Any code that would be broken by this needs to be broken, because it is almost certainly insecure in some way.

Good point here 👆

[VoR0220]

I think @PeterBorah makes a good point here. I think backwards compatibility for this security flaw might be worth breaking just this time.

[pipermerriam]

So far the only times I've used tx.origin are:

1. Bypassing stack depth checking
2. Source of entropy.

[slothbag]

I have contracts that ripple calls up a chain of contracts.. and I need to know which account was the originator. I use tx.origin for that.

[PeterBorah]

@slothbag Regardless of the outcome of this pull request, double and triple check that you're actually secure there. Chances are, there are ways for malicious contracts to do unexpected things by impersonating your users or your contracts. Is it ok if an attacker is able to call any of your functions at any time, with any arguments, while appearing to be a legitimate user? If not, tx.origin isn't safe.

Also consider whether it's ok that your users can't use multisig wallets, and can't be DAOs or anything like that.

If you're interested, I'd be happy to take a look at your contracts and offer advice for structuring your contracts so you don't need tx.origin. I am on a quest to eliminate it from existence!

[slothbag]

Thanks @PeterBorah I will definitely revisit my usage of it.

[Nashatyrev]

One of our contracts allows registering only non-contract address. We are checking this with msg.sender == tx.origin

[PeterBorah]

@Nashatyrev: Why do you want to do this? It vastly reduces your user's security options, because they can't use multisig, key revocation, or other contract-based security systems. As a user, I would not appreciate being forced to use a dangerous method (a bare private/public keypair) for interacting with a contract.

Additionally, this won't work the way you expect it to after EIP 101.