Skip to content

Conversation

@mrzeszutko
Copy link
Contributor

@mrzeszutko mrzeszutko commented Jan 8, 2026

Summary

Add semantic validation to P2P message deserialization to reject protocol-violating array/string lengths. This prevents DoS attacks via maliciously crafted messages that specify enormous sizes, which could cause memory exhaustion before the buffer range check catches the issue.

Changes

Foundation Layer

  • BufferReader: Added optional maxSize parameter to readVector(), readBuffer(), and readString() methods. This is a non-breaking change - all 161+ existing calls continue to work unchanged.

Constants Organization

  • stdlib/deserialization: New module with shared constants:

    • MAX_TXS_PER_BLOCK = 2 ** 16 (65536)
    • MAX_COMMITTEE_SIZE = 2,048 (theoretical max from bitmap design: 256 bytes × 8 bits)
    • MAX_BLOCKS_PER_CHECKPOINT = 72
    • MAX_TX_EFFECTS_PER_BODY = BLOBS_PER_CHECKPOINT * FIELDS_PER_BLOB
    • MAX_BLOCK_HASH_STRING_LENGTH = 128
  • p2p/reqresp/constants: P2P-specific constants:

    • MAX_VERSION_STRING_LENGTH = 64
    • MAX_BLOCK_HASH_STRING_LENGTH = 128
    • Re-exports MAX_TXS_PER_BLOCK from stdlib

Bounds Checking Added To

File Validation Added
BlockProposal.fromBuffer txHashes, txs arrays
Checkpoint.fromBuffer blocks vector
PublishedCheckpoint.fromBuffer attestations, l1BlockHash
Body.fromBuffer txEffects vector
CheckpointedL2Block.fromBuffer attestations, l1BlockHash
PublishedL2Block.fromBuffer attestations, l1BlockHash
deserializeValidateBlockResult committee, attestors, attestations, reason string
StatusMessage.fromBuffer version string, block hash
BitVector.fromBuffer length field

Tests

  • Added 9 new tests for BufferReader maxSize bounds checking
  • Added 2 tests for BitVector bounds validation

Fixes A-273

@mrzeszutko mrzeszutko changed the title Adding lenght validation on deserialization fix: adding lenght validation on deserialization Jan 8, 2026
@mrzeszutko mrzeszutko force-pushed the feature/bitvector-semantic-validation branch from c73a4ea to b6cc9cb Compare January 8, 2026 14:08
@PhilWindle PhilWindle changed the title fix: adding lenght validation on deserialization fix: adding length validation on deserialization Jan 8, 2026
Copy link
Collaborator

@PhilWindle PhilWindle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just one thing we need to check.

const reader = BufferReader.asReader(buffer);
return new StatusMessage(
reader.readString(), // compressedComponentsVersion
reader.readString(MAX_VERSION_STRING_LENGTH), // compressedComponentsVersion
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just check this? This version isn't a semver (i.e. 1.2.3-alpha.123). It's our compressed components version which incorporates a number of items. Can you verify that MAX_VERSION_STRING_LENGTH is sufficient for this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is an example:

p2p:7:libp2p_service:libp2p_service Started libp2p service with protocol version 00-31337-00000000-0-14d6d44e-01eff751

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be fine as MAX_VERSION_STRING_LENGTH is 64, but would like to check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants