Problem:
Token approval can be exploited by a seemingly legit proxy smart contract, whose underlying implementation contract is then upgraded to a malicious contract to take all the approved tokens.
Example:
Alice approves an upgradable proxy contract (UNI of her tokens (e.g. ERC20, ERC721, ERC1155) after evaluating the implementation contract.
The implementation contract can then be upgraded to a malicious contract and takes all Alice’s tokens
Solution:
Token approval carries the address of the underlying implementation contract, which if changed must ask for token approval again.
Correct. The act of marking a specific smart contract as “approved” to interact with your tokens means you trust all aspects of what it is; all functions, including any ability it has to upgrade itself. If it can upgrade itself (via standard proxy means, or any other custom logic upgrade option), then the user has to trust the governance process that contract has around choosing when/if to upgrade.
It is an attack vector if a malicious contract author can get a user’s trust to “approve” their malicious contract. There’s many ways a contract can be malicious that could be hard to detect automatically. If a contract uses ERC1967 as the means of being upgrade-able, that could be automatically detected by wallet software and add a notice when it popped up a “do you want to approve this contract?” transaction approval prompt. But an attacker could make their token upgrade-able by any other custom means and thwart that detection.
If that structure is desired, the creator of an upgrade-able contract could design it such that the logic contract is the one that the user grants approval to, and could be a way for an application team to show they have “nothing up their sleeves” to gain trust with the potential end-users of their application.
ERC1967 can help detect the changes in the underlying implementation contract but it can be too late for the accounts having given it approval to act upon it as the malicious author can enact an attack right after the upgrade.
When looking at this exploitation risk at large, I’m very concerned of the indefinite length of this exposed attack vector that every token approval given out can be a timed bomb in the future.
e.g. The majority of the current DeFi protocols need token approvals for an account interacting with it (e.g. Uniswap, Compound, Curve, and many many smaller DeFi protocols). If any of there protocols becomes malicious intentionally or due to a hack, all the users who have interacted with the protocol can lose all their tokens. (especially the case for ERC1155, where unscoped approval can lead to total loss of all the tokens held)
Using a rudimentary game theory analysis from three perspectives
(1) The upgradable proxy contract (e.g. DeFi protocol) would exploit the attack vector as long as
the value of all approved tokens (whether locked in the protocol or not) is greater than the cumulative future profit from the protocol
(2) The individual who has the access to upgrade the implementation contract (or the minimum number of admins when a multi-sig is used) would exploit the attack vector as long as the value of all the approved tokens (whether locked in the protocol or not) is greater than his/her own stake in the protocol + his reputation value
(3) End users would have to “ABSOLUTELY” trust a smart contract (DeFi protocol), no matter how much TVL it has, before interacting with it when a token approval is needed as one smart contract can potentially drain all the tokens being approved from an end user.
In regards to the implementation
As much as I want to keep the ERC token standards as they are since the problem is raised by upgradable proxy, I think we have to change how token approval works in the first place.
This is true of granting Approval to any smart contract; you need to absolutely trust all aspects of the smart contract you’re approving. That includes trusting if it’s upgrade-able but not limited to that. You claim that the issue is due to “upgradable proxies”, but if a user is cavalier with their token approvals, there’s many other attack vectors for that.
One thing I think you’re discounting is that the ERC20 standard allows for granting approval only for a specific quantity of tokens, and it is a good best-practice for users interacting with automated DeFi systems to only approve the amount of tokens they’re swapping at that time. Users who don’t follow that best-practice (out of convenience; not needing to grant approval again when they interact with the service in the future) then are accepting a risk with that.
I believe one of the core principles of blockchain is its trustless characteristic where we don’t need to trust anybody but the code. It is not a big problem that an account trusts a non-upgradable contract after carefully read the smart contracts and make sure the contract can not be exploited. The current upgradable proxy pattern makes it impossible to trust any upgradable code (contracts).
I also think upgradability is needed to fix bugs and improve a smart contract as time goes. The solution I was looking for is really a way to trust an upgradable smart contract as it is and when it upgrades itself, one has the option to opt out. Using an analogy IRL, someone signs a legal contract with another party which at a later stage changes, I think it is reasonable for the person to read the modified legal contract and have the option to opt out if the person believes it is to his/her disadvantage. It would not make sense for someone to trust another party unconditionally (except government maybe) as it is equivalent to giving away all his/her rights.
After thinking about potential solutions for the problem, it seems impossible to give trust to a proxied contract and restrict it in any non-trivial way. (correct me if I’m wrong, current proxy pattern allows implementation contract to modify EVERYTHING except caller and block related info).
A possible improvement is to (1) standardize user giving limited token approval and the interacted contract using multicall to process user request or (2) use the token receive hook
I think if you have an issue with approving then this means you don’t trust the upgrade-nature of the contract. If you don’t trust the contract, then don’t interact with it. Contracts can always compete to structure themselves in safer ways, such as only giving the implementation contract approval rather than the proxy contract. You may now trust the contract if this is the case, since you can read the code of the implementation contract.
I feel normal on-chain forces would work out the best here – I don’t see any viable path to intervention. The contract itself needs to gain your trust, nothing can change that. There are many other ways a contract can lose your trust.
Remember that a proxy contract is itself just a normal contract. I believe the entire ecosystem would break down if we treated these contracts differently.
Thanks for your feedback!
I think it is fundamentally different in trusting a proxy contract and the implementation contract. You can trust the implementation contract as it is since you can read the code and it would never change. The “state” of the proxy contract, however, can be later modified to almost “anything” by the very human controlling it. The former is trustless but the latter is trusting a “third party”.
Thank you for your reply.
The problem I see is that although it may feel fundamentally different, there is really a gradual spectrum of how much the logic of a contract can be altered. A proxy allows full alteration by governance, but even without a proxy, many state variables may be altered by governance which significantly change the logic of the code. Technically, with enough configurable variables, you could make the configuration turing-complete which is essentially equivalent to a proxy. The more flexibility introduced in the contract, the less we can trust it, but I don’t believe we can just depend on a hard distinction between what we see as a proxy contract and a non-proxy contract. A proxy is just one method to mask how logic can change with time, but it is definitely not the only method.