Ethernaut Level 1 Fallback

I found a YouTube series that goes over the Ethernaut challenges from OpenZepplin that teach people more about writing secure smart contracts

Level 1 can be found here: https://ethernaut.openzeppelin.com/level/0x9CB391dbcD447E645D6Cb55dE6ca23164130D008

The YouTube video I watched about this level is here: https://www.youtube.com/watch?v=i-8cCDajPDQ

Even though I watched a YouTube video on how to beat the level it was still pretty interesting and I learned a couple of lessons from it.

Below is the code for the main contract of the level:

// SPDX-License-Identifier: MIT
pragma solidity ^0.6.0;

import '@openzeppelin/contracts/math/SafeMath.sol';

contract Fallback {

  using SafeMath for uint256;
  mapping(address => uint) public contributions;
  address payable public owner;

  constructor() public {
    owner = msg.sender;
    contributions[msg.sender] = 1000 * (1 ether);
  }

  modifier onlyOwner {
        require(
            msg.sender == owner,
            "caller is not the owner"
        );
        _;
    }

  function contribute() public payable {
    require(msg.value < 0.001 ether);
    contributions[msg.sender] += msg.value;
    if(contributions[msg.sender] > contributions[owner]) {
      owner = msg.sender;
    }
  }

  function getContribution() public view returns (uint) {
    return contributions[msg.sender];
  }

  function withdraw() public onlyOwner {
    owner.transfer(address(this).balance);
  }

  receive() external payable {
    require(msg.value > 0 && contributions[msg.sender] > 0);
    owner = msg.sender;
  }
}

The object is simple, become the owner of the contract and drain the funds of the contract. The vulnerability is in the receive() function. The way it is written, it lets anyone who calls it (by sending ETH to the contract without specifying a function in the data of the transaction) and who is already a contributor to the contract, become the owner. That is what the contributions[msg.sender] > 0 asserts essentially. If you haven't contributed to the contract then you won't get past that require statement, but if you have then you can get past it.

So the trick is simply to contribute to the contract using the contribute() function. After that call the receive function by sending money but not specifying a function in the transaction. Then you become the owner of the contract and can drain funds using the withdraw function.

Once you see it, its obvious. But starting at the contract before the video, I had no idea what to do.

What did I learn from this? Setting the owner of your contract is a point of vulnerability. If there is anything about transferring ownership of a contract that needs to written carefully and tested well. If there are other challenges that require you to become the owner of the contract in Ethernaut I would just start by reading the contract and finding everywhere that the owner gets set. Then figure out how can I get to that line of code with a transaction so that I become the owner of the contract. That must be the adversarial way to look at a contract ownership that you need to defend against.

The best thing to do it just use Ownable or AccessControl from OpenZeppelin. These are proven and audited ways of adding security controls to a smart contract. Ownable includes a transferOwnership function. I think setting the owner anywhere else in your contract is just asking for trouble. If you are implementing an owner in your contract then just let transferOwnership handle it and be done.

Shoutout to D-Squared on Youtube for the very accessible video breakdown of this level, it really helped me, can't recommend it enough.