[S-7] Missing Validations

Fantom-foundation · Dec 15, 2024 · 8ef035c · 8ef035c
1 parent 25ad4c0
commit 8ef035c
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 7 deletions.
diff --git a/contracts/sfc/NodeDriver.sol b/contracts/sfc/NodeDriver.sol
@@ -19,6 +19,7 @@ contract NodeDriver is OwnableUpgradeable, UUPSUpgradeable, INodeDriver {
 
     error NotNode();
     error NotBackend();
+    error ZeroAddress();
 
     /// Callable only by NodeDriverAuth (which mediates calls from SFC and from admins)
     modifier onlyBackend() {
@@ -45,6 +46,9 @@ contract NodeDriver is OwnableUpgradeable, UUPSUpgradeable, INodeDriver {
     function initialize(address _backend, address _evmWriterAddress, address _owner) external initializer {
         __Ownable_init(_owner);
         __UUPSUpgradeable_init();
+        if (_backend == address(0) || _evmWriterAddress == address(0)) {
+            revert ZeroAddress();
+        }
         backend = NodeDriverAuth(_backend);
         evmWriter = IEVMWriter(_evmWriterAddress);
     }

diff --git a/contracts/sfc/NodeDriverAuth.sol b/contracts/sfc/NodeDriverAuth.sol
@@ -20,6 +20,7 @@ contract NodeDriverAuth is OwnableUpgradeable, UUPSUpgradeable {
     error SelfCodeHashMismatch();
     error DriverCodeHashMismatch();
     error RecipientNotSFC();
+    error ZeroAddress();
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {
@@ -30,6 +31,9 @@ contract NodeDriverAuth is OwnableUpgradeable, UUPSUpgradeable {
     function initialize(address payable _sfc, address _driver, address _owner) external initializer {
         __Ownable_init(_owner);
         __UUPSUpgradeable_init();
+        if (_sfc == address(0) || _driver == address(0)) {
+            revert ZeroAddress();
+        }
         driver = NodeDriver(_driver);
         sfc = ISFC(_sfc);
     }

diff --git a/contracts/sfc/SFC.sol b/contracts/sfc/SFC.sol
@@ -170,6 +170,7 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version, ISFC {
     error ValidatorNotActive();
     error ValidatorDelegationLimitExceeded();
     error NotDeactivatedStatus();
+    error InvalidValidatorID();
 
     // requests
     error RequestExists();
@@ -263,6 +264,9 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version, ISFC {
     ) external initializer {
         __Ownable_init(owner);
         __UUPSUpgradeable_init();
+        if (nodeDriver == address(0) || _c == address(0)) {
+            revert ZeroAddress();
+        }
         currentSealedEpoch = sealedEpoch;
         node = NodeDriverAuth(nodeDriver);
         c = ConstantsManager(_c);
@@ -390,6 +394,10 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version, ISFC {
         bytes calldata pubkey,
         uint256 createdTime
     ) external onlyDriver {
+        if (validatorID == 0) {
+            revert InvalidValidatorID();
+        }
+        _validatePubkey(pubkey);
         _rawCreateValidator(
             auth,
             validatorID,
@@ -417,12 +425,7 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version, ISFC {
         if (msg.value < c.minSelfStake()) {
             revert InsufficientSelfStake();
         }
-        if (pubkey.length != 66 || pubkey[0] != 0xc0) {
-            revert MalformedPubkey();
-        }
-        if (pubkeyAddressToValidatorID[_pubkeyToAddress(pubkey)] != 0) {
-            revert PubkeyUsedByOtherValidator();
-        }
+        _validatePubkey(pubkey);
         _createValidator(msg.sender, pubkey);
         _delegate(msg.sender, lastValidatorID, msg.value);
     }
@@ -505,6 +508,9 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version, ISFC {
 
     /// Update consts address.
     function updateConstsAddress(address v) external onlyOwner {
+        if (v == address(0)) {
+            revert ZeroAddress();
+        }
         c = ConstantsManager(v);
         emit ConstantsManagerUpdated(v);
     }
@@ -1183,6 +1189,16 @@ contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version, ISFC {
         return address(uint160(uint256(keccak256(pubkey[2:]))));
     }
 
+    /// Validate pubkey.
+    function _validatePubkey(bytes calldata pubkey) internal view {
+        if (pubkey.length != 66 || pubkey[0] != 0xc0) {
+            revert MalformedPubkey();
+        }
+        if (pubkeyAddressToValidatorID[_pubkeyToAddress(pubkey)] != 0) {
+            revert PubkeyUsedByOtherValidator();
+        }
+    }
+
     /// Get current time.
     function _now() internal view virtual returns (uint256) {
         return block.timestamp;

diff --git a/test/SFC.ts b/test/SFC.ts
@@ -57,9 +57,11 @@ describe('SFC', () => {
 
   describe('Genesis validator', () => {
     beforeEach(async function () {
+      const validatorPubKey =
+        '0xc000a2941866e485442aa6b17d67d77f8a6c4580bb556894cc1618473eff1e18203d8cce50b563cf4c75e408886079b8f067069442ed52e2ac9e556baa3f8fcc525f';
       const validator = ethers.Wallet.createRandom();
       await this.sfc.enableNonNodeCalls();
-      await this.sfc.setGenesisValidator(validator.address, 1, validator.publicKey, Date.now());
+      await this.sfc.setGenesisValidator(validator.address, 1, validatorPubKey, Date.now());
       await this.sfc.deactivateValidator(1, 1 << 3);
       await this.sfc.disableNonNodeCalls();
     });