Summary

There is no checking whether the one who registered is a Person (EOA) or a Smart Contract, also there is no checking for registration time limit.

Vulnerability Details

ThePredicter.sol::register()is a function where a User can register to become a Player by paying the entranceFee. However, the function itself doesn't check if the subject, who is registering is a "real person" or a Smart Contract, leading to a possible occurrence of a Reentrancy Attack.

Impact

The impact of the missing validation in this function can lead to many scenarios, 2 of the scenarios that are important to note are:

• Smart Contract can also register as a Player, which opens an opportunity to do exploits that require smart contracts such as reentrancy.

• A user can deploy a multiple smart contract to interact and register, potentially filling out all the remaining 14 slots (or 13 smart contracts + 1 EOA), and if all the accounts are verified (approved), the original EOA / user may have a bigger chance to win and add more prediction to the match.

Proof-of-Concept (PoC)

Scenario #1: Smart Contract can register as a Player

Using Foundry:

1. Create a simple smart contract that will interact with ThePredicter.sol

2. Here is a simple one

   // SPDX-License-Identifier: MIT

   pragma solidity 0.8.20;

   ​

   import "../ThePredicter.sol";

   import "forge-std/console.sol";

   ​

   contract testRegister{

   ThePredicter thePredicter;

   constructor(address _target) {

   thePredicter = ThePredicter(_target);

   }

   ​

   function register() public payable {

   thePredicter.register{value: msg.value}();

   }

   ​

   function ensureIsRegistered() public view returns(uint256){

   return uint256(thePredicter.playersStatus(address(this)));

   }

   ​

   }

• add the following code to the ThePredicter.test.sol

  import "../src/testing/testRegister.sol"

  ​

  address public tester_register = makeAddr("tester_register");

  ​

  /**

  REST OF CODE

  */

  ​

  function testFor_SmartContractRegistration() public {

  vm.startPrank(tester_register);

  vm.deal(tester_register, 5 ether);

  testregister = new testRegister(address(thePredicter));

  testregister.register{value: 0.04 ether}();

  vm.stopPrank();

  ​

  assertEq(testregister.ensureIsRegistered(), 1);

  }

  ​

  /**

  REST OF CODE

  */

• run forge test --mt testFor_SmartContractRegistration() -vvvv

Using REMIX IDE

• Deploy both smart contracts on REMIX

• create. a new Smart Contract to test the register()function, like this

  // SPDX-License-Identifier: MIT

  pragma solidity 0.8.20;

  ​

  import "../ThePredicter.sol";

  import "forge-std/console.sol";

  ​

  contract testRegister{

  ThePredicter thePredicter;

  constructor(address _target) {

  thePredicter = ThePredicter(_target);

  }

  ​

  function register() public payable {

  thePredicter.register{value: msg.value}();

  }

  ​

  function ensureIsRegistered() public view returns(uint256){

  return uint256(thePredicter.playersStatus(address(this)));

  }

  ​

  }

• Call the function register()to test the function and supply it with the correct entranceFee

• call the ensureIsRegistered()function and ensure that the value is 1

Scenario #2: A User deploys 13 Smart Contracts and also does the register function.

NOTE: This scenario is only valid if the organizeraccidentally marked all 14 registrations (13 Smart Contracts and Smart Contract Owner Account) as approved

• Create a Smart Contract that can interact with the ThePredicter.soland Register, such as below

  // SPDX-License-Identifier: MIT

  pragma solidity 0.8.20;

  ​

  import "../ThePredicter.sol";

  import "forge-std/console.sol";

  ​

  contract testRegister{

  ThePredicter thePredicter;

  constructor(address _target) {

  thePredicter = ThePredicter(_target);

  }

  ​

  function register() public payable {

  thePredicter.register{value: msg.value}();

  }

  ​

  }

• Add this to the ThePredicter.test.sol

  // REST OF CODE

  ​

  address[] public testRegisterDeployedAddress;

  ​

  // REST OF CODE

  ​

  function testFor_MultipleSCRegistration() public{

  // Organizer (Player and Address Organizer)

  vm.startPrank(organizer);

  vm.deal(organizer, 1 ether);

  thePredicter.register{value: 0.04 ether}();

  thePredicter.approvePlayer(address(organizer));

  vm.stopPrank();

  ​

  // Organizer friends (15 player)

  for (uint i = 0 ; i < 15; i++){

  // Make Ivan's Friend

  address user = makeAddr(Strings.toString(i));

  vm.deal(user, 1 ether);

  // Ivan's Friend Register

  vm.startPrank(user);

  thePredicter.register{value: 0.04 ether}();

  vm.stopPrank();

  // Ivan Approve his friend as a Player

  vm.startPrank(organizer);

  thePredicter.approvePlayer(address(user));

  vm.stopPrank();

  }

  ​

  // Register as the User

  vm.startPrank(tester_register);

  vm.deal(tester_register, 10 ether);

  thePredicter.register{value: 0.04 ether}();

  vm.stopPrank();

  vm.startPrank(organizer);

  thePredicter.approvePlayer(tester_register);

  vm.stopPrank();

  // Smart Contract For Register

  for (uint i = 0 ; i < 13; i++){

  // Give tester Register some ether

  vm.deal(tester_register, 1 ether);

  // Deploy new Smart Contract

  vm.startPrank(tester_register);

  testregister = new testRegister(address(thePredicter));

  testRegisterDeployedAddress.push(address(testregister));

  testregister.register{value: 0.04 ether}();

  vm.stopPrank();

  // Ivan Approve the Smart contract

  vm.startPrank(organizer);

  thePredicter.approvePlayer(address(testregister));

  vm.stopPrank();

  }

  ​

  // Test for extra user (30th User) - Will Fail

  address extraUser = makeAddr("extraUser");

  vm.deal(extraUser, 1 ether);

  vm.startPrank(extraUser);

  thePredicter.register{value: 0.04 ether}();

  vm.stopPrank();

  ​

  // Organizer trying to approve

  vm.startPrank(organizer);

  vm.expectRevert(ThePredicter.ThePredicter__AllPlacesAreTaken.selector);

  thePredicter.approvePlayer(extraUser);

  vm.stopPrank();

  ​

  // Make sure that 30 slot is already filled

  // Check that the player with index 0 is the organizer

  // Check that the player with index 16 is the tester_register

  // Check that the rest (17 - 29) are filled with Smart Contract

  for (uint i = 0 ; i < 15 ; i++){

  address user = makeAddr(Strings.toString(i));

  assertEq(uint256(thePredicter.playersStatus(address(user))),2);

  }

  for (uint i=0 ; i < 13 ; i++){

  address toCheck = testRegisterDeployedAddress[i];

  assertEq(uint256(thePredicter.playersStatus(toCheck)), 2);

  }

  assertEq(thePredicter.players(0), address(organizer));

  assertEq(thePredicter.players(16), address(tester_register));

  assertEq(uint256(thePredicter.playersStatus(address(organizer))), 2);

  }

  ​

  // REST OF CODE

• run forge test --mt test_MultiplerSCRegistration() -vvvv

Tools Used

• Foundry

• REMIX IDE

Recommendations

Adding a verifier for verifying that the registration is an EOA is necessary to prevent the scenario mentioned above, to do this we can use the tx.originmethod, below is the suggested fix for the code

The suggested fix above will check whether the player candidate is an External Owner Account (EOA), with that check, it will also check for the tx.originwhich will point to an EOA, preventing a Smart Contract from registering.