Ethernaut Fallout with Solidity 0.5

Security Ethernaut
1

Hi @paulinablaszk

I have been looking at how to update museum level Fallout to Solidity 0.5
I needed to use the old version of Ownable and haven’t found a way to import an old version so I have created a local copy.

Let me know your thoughts?
I can also have a look at the other museum levels.

Fallout.sol

pragma solidity ^0.5.0;

import 'openzeppelin-solidity/contracts/math/SafeMath.sol';
// https://github.com/OpenZeppelin/openzeppelin-solidity/blob/v1.10.0/contracts/ownership/Ownable.sol
import './LegacyOwnable.sol';

contract Fallout is LegacyOwnable {
  
  using SafeMath for uint256;
  mapping (address => uint) allocations;

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

  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];
  }
}

LegacyOwnable.sol

pragma solidity ^0.5.0;


/**
 * @title Ownable
 * @dev The Ownable contract has an owner address, and provides basic authorization control
 * functions, this simplifies the implementation of "user permissions".
 */
contract LegacyOwnable {
  address public owner;


  event OwnershipRenounced(address indexed previousOwner);
  event OwnershipTransferred(
    address indexed previousOwner,
    address indexed newOwner
  );


  /**
   * @dev The Ownable constructor sets the original `owner` of the contract to the sender
   * account.
   */
  constructor() public {
    owner = msg.sender;
  }

  /**
   * @dev Throws if called by any account other than the owner.
   */
  modifier onlyOwner() {
    require(msg.sender == owner);
    _;
  }

  /**
   * @dev Allows the current owner to relinquish control of the contract.
   */
  function renounceOwnership() public onlyOwner {
    emit OwnershipRenounced(owner);
    owner = address(0);
  }

  /**
   * @dev Allows the current owner to transfer control of the contract to a newOwner.
   * @param _newOwner The address to transfer ownership to.
   */
  function transferOwnership(address _newOwner) public onlyOwner {
    _transferOwnership(_newOwner);
  }

  /**
   * @dev Transfers control of the contract to a newOwner.
   * @param _newOwner The address to transfer ownership to.
   */
  function _transferOwnership(address _newOwner) internal {
    require(_newOwner != address(0));
    emit OwnershipTransferred(owner, _newOwner);
    owner = _newOwner;
  }
}
1 Like
2

Hi @abcoathup !

Thank you! I am not sure if we could “save” this level without the “old” version of constructor, because it loses the original meaning… I think that for this particular round it would be the best to leave it somehow in Solidity 0.4.22
It would be great if you could check also other “museum” levels!

1 Like
3

Hi @paulinablaszk I will check out the other museum levels this week.

I had to use a classic constructor for Fallout and a modern constructor for Ownable to make it work.

1 Like
4

Hi @paulinablaszk

I updated to use the current OpenZeppelin Ownable, rather than having to use a Legacy version. This now passes the test.

Fallout.sol

pragma solidity ^0.5.0;

import 'openzeppelin-solidity/contracts/math/SafeMath.sol';
import 'openzeppelin-solidity/contracts/ownership/Ownable.sol';
contract Fallout is Ownable {
  
  using SafeMath for uint256;
  mapping (address => uint) allocations;

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

  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];
  }
}
2 Likes
5

Did you change something in Fallout.test.js or FalloutFactory.sol to pass all tests?
Because due to https://github.com/OpenZeppelin/ethernaut/pull/123#issuecomment-491735559 I didn’t use Ownable contract.
Please take a look at my last commit. Do you think it is ok like this? (it passes test)

1 Like
6

Hi @paulinablaszk I was testing the contract separately, so doesn’t pass the tests, sorry, my mistake here.

I think your last commit is fine. I had wanted to use Ownable but it feels like too much of a pain. It is nice to keep Fallout level rather than retire it.

1 Like
7

Hi @paulinablaszk

I updated the Fallout contract and the test below so that we can use Ownable.

The main issue was the TestUtils was getting the first event, whilst the logs needed to be filtered to get the LevelInstanceCreatedLog event to get the instance address.

Fallout.sol

Use internal _transferOwnership function of Ownable in “constructor”

pragma solidity ^0.5.0;

import 'openzeppelin-solidity/contracts/math/SafeMath.sol';
import 'openzeppelin-solidity/contracts/ownership/Ownable.sol';


contract Fallout is Ownable {
  
  using SafeMath for uint256;
  mapping (address => uint) allocations;


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

  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];
  }
}

Fallout.test.js

Update test to check for owner being level.instance first.

const FalloutFactory = artifacts.require('./levels/FalloutFactory.sol')
const Fallout = artifacts.require('./attacks/Fallout.sol')

const Ethernaut = artifacts.require('./Ethernaut.sol')
const { BN, constants, expectEvent, expectRevert } = require('openzeppelin-test-helpers')
import * as utils from '../utils/TestUtils'

contract('Fallout', function(accounts) {

  let ethernaut
  let level
  let owner = accounts[1]
  let player = accounts[0]

  beforeEach(async function() {
    ethernaut = await Ethernaut.new();
    level = await FalloutFactory.new()
    await ethernaut.registerLevel(level.address)
    console.log(ethernaut.address, level.address)
  });

  it('should allow the player to solve the level', async function() {

    const instance = await utils.createLevelInstance(
      ethernaut, level.address, player, Fallout,
      {from: player}
    )

    assert.equal(await instance.owner(), level.address)
      
    await instance.Fal1out()
    assert.equal(await instance.owner(), player)

    // Factory check
    const ethCompleted = await utils.submitLevelInstance(
      ethernaut,
      level.address,
      instance.address,
      player
    )
    assert.equal(ethCompleted, true)
  });

});

TestUtils.js

Update createLevelInstance function to filter for the LevelInstanceCreatedLog event to get the instance address.

export function getBalance(web3, address) {
  return new Promise(function(resolve, reject) {
    web3.eth.getBalance(address, function(error, result) {
      if(error) reject(error)
      else resolve(web3.utils.fromWei(result.toString(), 'ether'))
    })
  })
}

export function skipBlocks(numBlocks, web3) {
  return new Promise(async resolve => {
    for(let i = 0; i < numBlocks; i++) {
      await skipBlock(web3);
    }
    resolve();
  });
}

export function skipBlock(web3) {
  return new Promise((resolve, reject) => {
    web3.currentProvider.sendAsync({
      jsonrpc: '2.0',
      method: 'evm_mine'
    }, (error, result) => {
      if(error) reject();
      else resolve(result);
    });
  });
}

export async function createLevelInstance(ethernaut, levelAddress, player, levelInstanceClass, params) {
  return new Promise(async function(resolve, reject) {
    const data = params || {from: player}
    const tx = await ethernaut.createLevelInstance(levelAddress, data);
    if(tx.logs.length === 0) reject()
    else {
      const events = tx.logs.filter(e => e.event === "LevelInstanceCreatedLog");
      const instanceAddress = events[0].args.instance;
      const instance = await levelInstanceClass.at(instanceAddress);
      resolve(instance);
    }
  });
}

export async function submitLevelInstance(ethernaut, levelAddress, instanceAddress, player, params) {
  return new Promise(async function(resolve) {
    const data = params || {from: player}
    const tx = await ethernaut.submitLevelInstance(instanceAddress, data);
    if(tx.logs.length === 0) resolve(false)
    else {
      const log = tx.logs[0].args;
      const ethLevelAddress = log.level;
      const ethPlayer = log.player;
      if(player === ethPlayer && levelAddress === ethLevelAddress) {
        resolve(true)
      }
      else resolve(false)
    }
  });
}
2 Likes