#39812 [BC-Critical] Bypass `SetCertTime` transaction signature check #1

Submitted on Feb 8th 2025 at 00:38:00 UTC by @Blockian for Audit Comp | Shardeum: Core III

  • Report ID: #39812

  • Report Type: Blockchain/DLT

  • Report severity: Critical

  • Target: https://github.com/shardeum/shardeum/tree/bugbounty

  • Impacts:

    • Direct loss of funds

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

Description

Impact

Bypass the signature of SetCertTime transaction, which allows for draining any validators balance.

  • Note: same impact as report 33750 but a different root cause.

Root Cause

There is an inconsistency between the function isSetCertTimeTx and the function isInternalTx, specifically with regards to .tx.isInternalTx. This makes it so that SetCertTime transactions with .tx.isInternalTx = true don't undergo SetCertTime validation.

Attack Flow

A malicious user submits a SetCertTime transaction on behalf of someone else (nominator == victim), causing a fee to be deducted from their account. The malicious user can then continue doing so until the node is kicked / drained.

Deep Dive

When validating a SetCertTime transaction, the function validateTxnFields which is used to validate transaction fields calls isSetCertTimeTx to decide if it should call validateSetCertTimeTx which validates properties of the transaction, including its signer's identity .

isSetCertTimeTx returns false if tx.isInternalTx isn't set to true, even if tx.internalTXType === InternalTXType.SetCertTime:

When applying a SetCertTime transaction, the check is:

in order to call applySetCertTimeTx even if tx.isInternalTx isn't set to true, as long as isInternalTx returns true, which can happen if tx.tx.isInternalTx is true:

The fee is then subtracted from the victim's balance in applySetCertTimeTx here.

Suggested Fix

I suggested to do 3 things:

  1. Only use the isInternalTx function, never use .isInternalTx

  2. Only use the isSetCertTimeTx function, never compare internelTxType

  3. Remove the third check from isInternalTx as it is not used

Severity

This allows to kick validators and do drain funds, and so it critical. In addition, this is the same as 33750 which was classified as critical.

Proof of Concept

POC

  1. All of shardeum, core, and json-rpc-server should be on the bugbounty branch

  2. Apply debug-10-nodes as stated in the docs

  3. Apply the following patch on core (the logs are optional):

  1. Apply the following patch on shardeum

  1. Run the network with 10 nodes

  2. Wait for all the nodes to be active

  3. Run the following code, to create a staking account:

  1. Run the following code, to start draining the staking account:

NOTE: This POC uses a node as the attacker just because it is guaranteed to have an account and I know its private key. Any account would do.

Was this helpful?