Why is this delegatecall attack not working?

Support
1

I am confused by this simple contract. The main purpose is to bypass the OnlyContract() modifier.

Victim contract.

pragma solidity 0.7.6;

contract Victim {

    modifier OnlyContract() {
        require(msg.sender == address(this), "You are not this contract");
        _;
    }

    function withdraw(uint256 amount) external {
        (bool success,) = msg.sender.call{value : amount}("");
        require(success, "Withdraw failed");
    }

    function ContractWithdraw(address payable to_, uint256 value) external OnlyContract {
        require(address(this).balance >= value, "No funds");
        (bool success,) = to_.call{value : value}("");
        require(success, "Contract Withdraw failed");
    }
}

Malicious Contract

pragma solidity 0.7.6;

contract Malicious {
    address payable owner;

    constructor() public {
        owner = msg.sender;
    }
    
    receive() external payable {
        address x = address(0x0000000000);
        (bool success, bytes memory data) = x.delegatecall(abi.encodeWithSignature("ContractWithdraw(address,uint256)", address(this), 1000000000000000000));
    }
    
    function Attack() public {
        x.withdraw();
    }
    
    function withdraw_all() public {
        owner.transfer(address(this).balance);
    }
}

The flow is like this:

  1. Attacker call Attack() on Malicious contract.
  2. Victim contract send ETH to Malicious contract.
  3. Victim contract go into Malicious fallback contract and delegatecall to his own contract.
  4. From here, msg.sender delegatecall has changed to Victim contract.

But in reality, the delegate call transaction reverted.
I think I’ve bypassed the OnlyContract() modifier.

Why did the attack fail?

1 Like
2

Hi @tesgerburger,

Welcome to the community :wave:

Where is this challenge from? I didn’t recognize it.

3

Hello @tesgerburger

I don’t think you completelly understand the mechanism behind delegate call. Just as a reminder, when you do a delegateCall:

  • msg.sender doesn’t change
  • you execute code from the called contract
  • you execute it in the context of the caller contract.

This means that if the call you delegate to does read/write from storage, or does token transfer … it does it from the point of view of the caller. In particular, you cannot, though a delegate call, transfer assets (Eth, ERC20) that are owned by the called contract.

I’d be happy to walk you through what happens in your case, but I’m afraid your code is unclear, and doesn’t actually need a fancy attack to be vulnerable (the withdraw function by itself allows anyone to drain the Victim funds)

2 Likes