chore: hardening added for some params in /set-config and type checks…

… in updateConfig
shardeum · Jun 24, 2024 · d76993e · d76993e
1 parent 81f36ab
commit d76993e
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 45 deletions.
diff --git a/src/API.ts b/src/API.ts
@@ -919,20 +919,29 @@ export function registerRoutes(server: FastifyInstance<Server, IncomingMessage,
       },
     },
     async (_request: ConfigPatchRequest, reply) => {
+      const RESTRICTED_PARAMS = [
+        'ARCHIVER_IP',
+        'ARCHIVER_PORT',
+        'ARCHIVER_HASH_KEY',
+        'ARCHIVER_SECRET_KEY',
+        'ARCHIVER_PUBLIC_KEY',
+      ]
       try {
         const { sign, ...newConfig } = _request.body
         const validKeys = new Set(Object.keys(config))
         const payloadKeys = Object.keys(newConfig)
-        const invalidKeys = payloadKeys.filter((key) => !validKeys.has(key))
+        const invalidKeys = payloadKeys.filter(
+          (key) => !validKeys.has(key) || RESTRICTED_PARAMS.includes(key)
+        )
 
         if (invalidKeys.length > 0)
-          throw new Error(`Invalid config properties provided: ${invalidKeys.join(', ')}`)
+          throw new Error(`Invalid/Unauthorised config properties provided: ${invalidKeys.join(', ')}`)
 
         if (config.VERBOSE)
           Logger.mainLogger.debug('Archiver config update executed: ', JSON.stringify(newConfig))
 
         const updatedConfig = updateConfig(newConfig)
-        reply.send({ success: true, updatedConfig })
+        reply.send({ success: true, ...updatedConfig, ARCHIVER_SECRET_KEY: '' })
       } catch (error) {
         reply.status(400).send({ success: false, reason: error.message })
       }

diff --git a/src/Config.ts b/src/Config.ts
@@ -228,10 +228,12 @@ export async function overrideDefaultConfig(file: string): Promise<void> {
 
 export function updateConfig(newConfig: Partial<Config>): Config {
   for (const key in newConfig) {
-    if (newConfig[key] === 'true') newConfig[key] = true
-    else if (newConfig[key] === 'false') newConfig[key] = false
-    else if (typeof newConfig[key] !== 'boolean' && !Number.isNaN(Number(newConfig[key])))
-      newConfig[key] = Number(newConfig[key])
+    if (typeof newConfig[key] !== typeof config[key])
+      throw new Error(
+        `Value with incorrect type passed to update the Archiver Config: ${key}:${
+          newConfig[key]
+        } of type ${typeof newConfig[key]}`
+      )
   }
   config = merge(config, newConfig)
   return config

diff --git a/src/Data/Data.ts b/src/Data/Data.ts
@@ -536,49 +536,57 @@ export function addDataSender(sender: DataSender): void {
 }
 
 async function syncFromNetworkConfig(): Promise<any> {
-  // Define the query function to get the network config from a node
-  const queryFn = async (node): Promise<object> => {
-    const REQUEST_NETCONFIG_TIMEOUT_SECOND = 2 // 2s timeout
-    try {
-      const response = await P2P.getJson(
-        `http://${node.ip}:${node.port}/netconfig`,
-        REQUEST_NETCONFIG_TIMEOUT_SECOND
+  try {
+    // Define the query function to get the network config from a node
+    const queryFn = async (node): Promise<object> => {
+      const REQUEST_NETCONFIG_TIMEOUT_SECOND = 2 // 2s timeout
+      try {
+        const response = await P2P.getJson(
+          `http://${node.ip}:${node.port}/netconfig`,
+          REQUEST_NETCONFIG_TIMEOUT_SECOND
+        )
+        return response
+      } catch (error) {
+        Logger.mainLogger.error(`Error querying node ${node.ip}:${node.port}: ${error}`)
+        return null
+      }
+    }
+    // Define the equality function to compare two responses
+    const equalityFn = (responseA, responseB): boolean => {
+      return (
+        responseA?.config?.sharding?.nodesPerConsensusGroup ===
+        responseB?.config?.sharding?.nodesPerConsensusGroup
       )
-      return response
-    } catch (error) {
-      Logger.mainLogger.error(`Error querying node ${node.ip}:${node.port}: ${error}`)
-      return null
     }
-  }
-  // Define the equality function to compare two responses
-  const equalityFn = (responseA, responseB): boolean => {
-    return (
-      responseA?.config?.sharding?.nodesPerConsensusGroup ===
-      responseB?.config?.sharding?.nodesPerConsensusGroup
+    // Get the list of 10 max random active nodes or the first node if no active nodes are available
+    const nodes =
+      NodeList.getActiveNodeCount() > 0 ? NodeList.getRandomActiveNodes(10) : [NodeList.getFirstNode()]
+    // Use robustQuery to get the consensusRadius from multiple nodes
+    const tallyItem = await robustQuery(
+      nodes,
+      queryFn,
+      equalityFn,
+      3 // Redundancy (minimum 3 nodes should return the same result to reach consensus)
     )
-  }
-  // Get the list of 10 max random active nodes or the first node if no active nodes are available
-  const nodes =
-    NodeList.getActiveNodeCount() > 0 ? NodeList.getRandomActiveNodes(10) : [NodeList.getFirstNode()]
-  // Use robustQuery to get the consensusRadius from multiple nodes
-  const tallyItem = await robustQuery(
-    nodes,
-    queryFn,
-    equalityFn,
-    3 // Redundancy (minimum 3 nodes should return the same result to reach consensus)
-  )
 
-  if (tallyItem?.value?.config) {
-    // Updating the Archiver Config as per the latest Network Config
-    const devPublicKeys = tallyItem.value.config.devPublicKeys
-    const updateConfigProps = {
-      newPOQReceipt: tallyItem.value.config.useNewPOQ,
-      DevPublicKey: Object.keys(devPublicKeys).find((key) => devPublicKeys[key] === 3),
+    if (tallyItem?.value?.config) {
+      // Updating the Archiver Config as per the latest Network Config
+      const devPublicKeys = tallyItem.value.config?.devPublicKeys
+      const updateConfigProps = {
+        newPOQReceipt: tallyItem.value.config?.useNewPOQ,
+        DevPublicKey:
+          devPublicKeys && Object.keys(devPublicKeys).length >= 3
+            ? Object.keys(devPublicKeys).find((key) => devPublicKeys[key] === 3)
+            : '',
+      }
+      updateConfig(updateConfigProps)
+      return tallyItem
     }
-    updateConfig(updateConfigProps)
-    return tallyItem
+    return null
+  } catch (error) {
+    Logger.mainLogger.error('❌ Error in syncFromNetworkConfig: ', error)
+    return null
   }
-  return null
 }
 
 async function getConsensusRadius(): Promise<number> {
@@ -590,6 +598,16 @@ async function getConsensusRadius(): Promise<number> {
   if (tallyItem?.value?.config) {
     nodesPerEdge = tallyItem.value.config.sharding.nodesPerEdge
     nodesPerConsensusGroup = tallyItem.value.config.sharding.nodesPerConsensusGroup
+
+    if (!Number.isInteger(nodesPerConsensusGroup) || nodesPerConsensusGroup <= 0) {
+      Logger.mainLogger.error('nodesPerConsensusGroup is not a valid number:', nodesPerConsensusGroup)
+      return currentConsensusRadius
+    }
+
+    if (!Number.isInteger(nodesPerEdge) || nodesPerEdge <= 0) {
+      Logger.mainLogger.error('nodesPerEdge is not a valid number:', nodesPerEdge)
+      return currentConsensusRadius
+    }
     // Upgrading consensus size to an odd number
     if (nodesPerConsensusGroup % 2 === 0) nodesPerConsensusGroup++
     const consensusRadius = Math.floor((nodesPerConsensusGroup - 1) / 2)

diff --git a/src/ShardFunctions.ts b/src/ShardFunctions.ts
@@ -52,7 +52,7 @@ class ShardFunctions {
 
     //make sure nodesPerConsenusGroup is an odd number >= 3
     if (nodesPerConsenusGroup % 2 === 0 || nodesPerConsenusGroup < 3) {
-      throw new Error(`nodesPerConsenusGroup:${nodesPerConsenusGroup} must be odd and >= 3`)
+      throw new Error(`nodesPerConsenusGroup: ${nodesPerConsenusGroup} must be odd and >= 3`)
     }
 
     shardGlobals.consensusRadius = Math.floor((nodesPerConsenusGroup - 1) / 2)