Skip to content

Conversation

@Alfredooe
Copy link

@Alfredooe Alfredooe commented Jan 3, 2026

image

Negative Hops Relay Amplification

Relay condition validation allowed negative values, by sending -100 you can cause 102 relay hops. Observed during testing that setting -100 would cause a massive increase in duplicate messages received back from testnet.

validateMessage is loosely validating

This one doesn't seem impacting but adding additional validation here seems good. Could reduce performance impact of attempted abuse by rejecting earlier.

Weak PoW Mitigation

Assuming you can't change the PoW to 00000 from 0000, I was able to generate 100,000 Identities and broadcast these into a testnet easily with a cheap VPS.

An easy win fix here seems to be introducing a rate limit on the number of peers accepted over a single connection, This is a suggestion and could be tailored to network rate. Setting this to 1000 new peers over a 5s window limit the impact without raising the PoW or currently limiting the dopamine network.

Sybil attack PoC for this can be seen in base branch of my fork repo as troll_parallel_v2.js

Give all of these a proper test, I'm terrible at Javascript.

Copilot AI review requested due to automatic review settings January 3, 2026 22:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses critical security vulnerabilities in the P2P messaging system, including a negative hops amplification attack, loose message validation, and Sybil attack vectors through weak proof-of-work.

  • Fixes negative hops relay amplification by validating hops >= 0 before relaying messages
  • Strengthens message validation with strict type checking and integer validation for all numeric fields
  • Implements per-connection rate limiting (1000 new peers per 5 seconds) to mitigate Sybil attacks exploiting weak PoW

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
src/config/constants.js Adds rate limiting constants (RATE_LIMIT_WINDOW and RATE_LIMIT_MAX_NEW_PEERS) for Sybil attack mitigation
src/state/diagnostics.js Adds rateLimitedConnections counter to track rate limit enforcement
src/p2p/messaging.js Implements negative hops validation, per-socket rate limiting for new peers, and enhanced validateMessage with strict type checks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@emily-is-my-username
Copy link

emily-is-my-username commented Jan 4, 2026

This one doesn't seem impacting but adding additional validation here seems good.

Sending the id as an array of integers instead of the hex string creates valid messages with valid signatures btw because adding array + string (for the id + nonce hash) is a valid javascript thing to do and Buffer.from(id, "hex") just ignores the "hex" encoding if you pass an array, reconstructing a valid public key.

Those arrays are used as Map keys and could break some stuff (because they hash by object, not content), but that currently does not work because HyperLogLog fails when calling charCodeAt on the id before it is inserted into any map.

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.

2 participants