fix: prevent cheating in Preservation (challenge 16)

[leovct]

Description

Make sure that the addresses passed to the constructor of Preservation implement the LibraryContract. Otherwise, the hack is trivial, all you need is to pass the address of a contract that has the same storage layout as the Preservation contrat and that overrides the owner value when calling the setTime method.

// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.0;

import '../../src/EthernautCTF/Preservation.sol';
import '@forge-std/Test.sol';
import '@forge-std/console2.sol';

contract Helper {
  // Same storage layout as the Preservation contract.
  address public timeZone1Library;
  address public timeZone2Library;
  address public owner;

  function setTime(uint256) public {
    owner = msg.sender;
  }
}

contract PreservationNaiveExploit is Test {
  Helper library1;
  Helper library2;
  Preservation target;

  address deployer = makeAddr('deployer');
  address exploiter = makeAddr('exploiter');

  function setUp() public {
    vm.startPrank(deployer);
    library1 = new Helper();
    library2 = new Helper();
    target = new Preservation(address(library1), address(library2));
    console2.log('Target contract deployed');
    vm.stopPrank();
  }

  function testExploit() public {
    address owner = target.owner();
    console2.log('Current owner: %s', owner);
    assertEq(owner, deployer);

    vm.startPrank(exploiter);
    target.setFirstTime(0); // dummy value.
    vm.stopPrank();

    owner = target.owner();
    console2.log('New owner: %s', owner);
    assertEq(owner, exploiter);
  }
}

Without the fix:

$ forge test -vvv --match-contract PreservationNaive
[⠊] Compiling...
[⠃] Compiling 1 files with Solc 0.8.26
[⠊] Solc 0.8.26 finished in 817.27ms
Compiler run successful!

Ran 1 test for test/EthernautCTF/PreservationNaiveExploit.t.sol:PreservationNaiveExploit
[PASS] testExploit() (gas: 29493)
Logs:
  Target contract deployed
  Current owner: 0xaE0bDc4eEAC5E950B67C6819B118761CaAF61946
  New owner: 0x5Bf3eeB5560eEACC941C553320999006D27dD42b

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 9.43ms (720.67µs CPU time)

Ran 1 test suite in 145.11ms (9.43ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

With the fix:

$ forge test -vvv --match-contract PreservationNaiveFixed
Compiler run failed:
Error (9553): Invalid type for argument in function call. Invalid implicit conversion from contract Helper to contract LibraryContract requested.
  --> test/EthernautCTF/PreservationNaiveExploitFixed.t.sol:31:32:
   |
31 |     target = new Preservation2(library1, library2);
   |                                ^^^^^^^^

Error (9553): Invalid type for argument in function call. Invalid implicit conversion from contract Helper to contract LibraryContract requested.
  --> test/EthernautCTF/PreservationNaiveExploitFixed.t.sol:31:42:
   |
31 |     target = new Preservation2(library1, library2);
   |                                          ^^^^^^^^

Error: 
Compilation failed

[GianfrancoBazzani]

Hello @leovct, thanks for suggesting these changes; nonetheless, there are a couple of caveats here:

1. The game is meant to be played on-chain. As far as I understand, you recreated the test locally by deploying custom libraries (Helper from your code snippet). This won't be the case on-chain, as the instance is deployed by the Ethernaut level's factory, and these libraries are set to the LibraryContract there. Please check this lines.

2. In Solidity, any custom contract (or interface) type essentially acts as a wrapper around the address type. This allows you to call methods on the address without needing to use low-level calls. However, Solidity does not ensure that a given address actually implements the methods of the specified contract. You cannot guarantee that addresses passed to the constructor of Preservation implement the functions of LibraryContract. Solidity lets you assign any arbitrary address to a contract type, but it does not check if the code at that address matches the specified interface or contract definition.

I proceed to close this PR.