Ethernaut Level 2 Fallout

In my post for Ethernaut Level 1 Fallback I mentioned:

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.

Well that was a good lesson to learn for Level 2.

Here is the level itself: https://ethernaut.openzeppelin.com/level/0x5732B2F88cbd19B6f01E3a96e9f0D90B917281E5

Here is the contract for the level:

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

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

contract Fallout {

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


  /* constructor */
  function Fal1out() public payable {
    owner = msg.sender;
    allocations[owner] = msg.value;
  }

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

  function allocate() public payable {
    allocations[msg.sender] = allocations[msg.sender].add(msg.value);
  }

  function sendAllocation(address payable allocator) public {
    require(allocations[allocator] > 0);
    allocator.transfer(allocations[allocator]);
  }

  function collectAllocations() public onlyOwner {
    msg.sender.transfer(address(this).balance);
  }

  function allocatorBalance(address allocator) public view returns (uint) {
    return allocations[allocator];
  }
}

There is only one line in this contract where owner of the contract is set and its in the Fal1out(), which is a misspelling of the contract name Fallout. The /* constructor */ is meant as misdirection. This isn't a constructor because it doesn't match the contract name and it is publicly callable. Normally the constructor can only be called when the contract is deployed but because this isn't a constructor, Fal1out() can be called anytime. So all you have to do is send a transaction to this function for Fal1out() with some ETH (since its payable) and you get set as the owner.

I was initially a bit confused though, because I am used to constructors using the constructor keyword. It turns out that an upgrade was made in solidity to make this change that constructors should just use the constructor keyword. This was done because this was the source of an actual Mainnet bug. Where someone changed the name of their project and their name of their contract, but forget to change the name of their constructor. Pretty crazy, but with the update to Solidity to use the constructor keyword for defining a constructor, that should remove that as something to really worry about when writing a smart contract. Still useful to learn about, and more generally its a good reminder to think when you are naming your contracts and functions, and also to think when you renaming anything (functions, contracts or variables) as doing so carelessly can introduce bugs.

I also watched this YouTube video that provided more context and that I definitely recommend: https://www.youtube.com/watch?v=VGbxxdhOWvU