Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential for resource exhaustion vulnerability in case of a byzantine Execution Node #6899

Open
AlexHentschel opened this issue Jan 15, 2025 · 1 comment
Labels
Epic Execution Cadence Execution Team Preserve Stale Bot repellent Protocol Team: Issues assigned to the Protocol Pillar. S-BFT

Comments

@AlexHentschel
Copy link
Member

AlexHentschel commented Jan 15, 2025

Problem description

Please take a look at the DefaultSerializer in module/executiondatasync/execution_data/serializer.go

It defines its own encoding without the cbor flag cbor.ExtraDecErrorUnknownField (see networking layer's decoder configuration for comparison). My understanding is that the DefaultSerializer not strict in that it would just ignore data in the decoding step that is for an unknown struct field.
In PR #6879, we have added the following warning to the DefaultSerializer:

// DefaultSerializer is the default implementation for an Execution Data serializer.
// It is configured to use cbor encoding with LZ4 compression.
// 
// CAUTION: this encoding should only be used for encoding/decoding data within a node.
// If used for decoding data that is shared between nodes, it makes the recipient VULNERABLE
// to RESOURCE EXHAUSTION ATTACKS, where a byzantine sender could include garbage data in the
// encoding, which would not be noticed by the recipient because the garbage data is dropped 
// at the decoding step - yet, it consumes the recipient's networking bandwidth. 
var DefaultSerializer Serializer

Though, the problem remains that the usage of the DefaultSerializer in our code base is potentially unsafe and may expose a variety of nodes roles to undetectable resource exhaustion attacks.

Details on areas that may be vulnerable to resource exhaustion attacks

I haven't scanned through the code entirely and properly analyzed every usage, but based on a preliminary investigation, I think we may have major vulnerabilities for a resource exhaustion attacks in our code base 😱 ... At least, there is no argument provided in the code, why the implementation is not vulnerable, which would be the required level of rigour. Here is my understanding:

  • The DefaultSerializer is used in a variety of places Screenshot 2025-01-15 at 3 05 41 PM
  • My understanding is that the DefaultSerializer is used ExecutionDataStore, which in turn is used for chopping the execution data into blobs and sharing that data via bitswap with other nodes (?)
  • So a byzantine execution node could include a bunch of garbage data in the serialized data structure, which would be dropped by the recipient during the decoding and hence not noticed. Though, a byzantine data provider could easily consume the recipients entire networking bandwidth and thereby render it unresponsive.

Nodes that could be vulnerable:

  • verification nodes (receiving execution data)
  • access nodes (receiving execution data)
  • observer nodes (receiving execution data)

Nodes that could potentially mount the attack:

  • byzantine Execution Node
  • honest Verification, Access, Observer and Archive Nodes might unknowingly amplify an attack (if they nodes are authorized to provide data blobs, which is desired to reduce load from Execution Nodes where this data originates from), by sharing bloated data blobs

Definition of Done

This is an epic. The issue requires further investigation and scoping.

  1. investigate all usages of DefaultSerializer (outside of tests)
  2. fix usages that are vulnerable to resource exhaustion attacks
  3. For every usages of DefaultSerializer that are not vulnerable, a conclusive argument must be provided in the code explaining why the implementation is not vulnerable. A strong argument of byzantine-fault-tolerance is the required level of rigour for a high-assurance BFT software system. Absence of known vulnerabilities if insufficient.

I want to emphasize that I don't like that the word "default" is in the name of the DefaultSerializer struct. This is because components used by "default" should be BFT, which is not the case for the DefaultSerializer (principle of "Safety By Default").

@AlexHentschel AlexHentschel added Epic Execution Cadence Execution Team Preserve Stale Bot repellent Protocol Team: Issues assigned to the Protocol Pillar. S-BFT labels Jan 15, 2025
@fxamacker
Copy link
Member

Here is a bit more info for people looking into using CBOR and security considerations.

As mentioned last year in my comment to @AlexHentschel on Feb 23, 2024:

The CBOR codec we use has configurable settings for security, deterministic encoding, etc.

The README of https://github.com/fxamacker/cbor has more info about security considerations, cbor settings, and trade-offs (speed vs security, etc.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic Execution Cadence Execution Team Preserve Stale Bot repellent Protocol Team: Issues assigned to the Protocol Pillar. S-BFT
Projects
None yet
Development

No branches or pull requests

2 participants