From 34ee6a4ae9faeea334b715643f633a9d267aa971 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Tue, 17 Mar 2026 11:33:20 -0300 Subject: [PATCH] fix(p2p): penalize peers for errors during response reading Errors in readMessage (invalid status bytes, oversized snappy responses, corrupt data) were caught and silently converted to UNKNOWN status returns. Since sendRequestToPeer only calls handleResponseError in its own catch block, none of these errors resulted in peer penalties. The request was simply retried with another peer, allowing a malicious peer to waste bandwidth indefinitely. Co-Authored-By: Claude Opus 4.6 (1M context) --- yarn-project/p2p/src/services/encoding.ts | 10 ++++++- .../p2p/src/services/reqresp/reqresp.ts | 28 ++++++++++++------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/yarn-project/p2p/src/services/encoding.ts b/yarn-project/p2p/src/services/encoding.ts index 0aea0032d158..d21fbe695597 100644 --- a/yarn-project/p2p/src/services/encoding.ts +++ b/yarn-project/p2p/src/services/encoding.ts @@ -9,6 +9,14 @@ import { webcrypto } from 'node:crypto'; import { compressSync, uncompressSync } from 'snappy'; import xxhashFactory from 'xxhash-wasm'; +/** Thrown when a Snappy-compressed response exceeds the allowed decompressed size. */ +export class OversizedSnappyResponseError extends Error { + constructor(decompressedSize: number, maxSizeKb: number) { + super(`Decompressed size ${decompressedSize} exceeds maximum allowed size of ${maxSizeKb}kb`); + this.name = 'OversizedSnappyResponseError'; + } +} + // Load WASM const xxhash = await xxhashFactory(); @@ -86,7 +94,7 @@ export class SnappyTransform implements DataTransform { const { decompressedSize } = readSnappyPreamble(data); if (decompressedSize > maxSizeKb * 1024) { this.logger.warn(`Decompressed size ${decompressedSize} exceeds maximum allowed size of ${maxSizeKb}kb`); - throw new Error(`Decompressed size ${decompressedSize} exceeds maximum allowed size of ${maxSizeKb}kb`); + throw new OversizedSnappyResponseError(decompressedSize, maxSizeKb); } return Buffer.from(uncompressSync(data, { asBuffer: true })); diff --git a/yarn-project/p2p/src/services/reqresp/reqresp.ts b/yarn-project/p2p/src/services/reqresp/reqresp.ts index bd15d5ff6c4e..38354e1dd67f 100644 --- a/yarn-project/p2p/src/services/reqresp/reqresp.ts +++ b/yarn-project/p2p/src/services/reqresp/reqresp.ts @@ -16,7 +16,7 @@ import { IndividualReqRespTimeoutError, InvalidResponseError, } from '../../errors/reqresp.error.js'; -import { SnappyTransform } from '../encoding.js'; +import { OversizedSnappyResponseError, SnappyTransform } from '../encoding.js'; import type { PeerScoring } from '../peer-manager/peer_scoring.js'; import { DEFAULT_INDIVIDUAL_REQUEST_TIMEOUT_MS, @@ -553,16 +553,10 @@ export class ReqResp implements ReqRespInterface { data: message, }; } catch (e: any) { + // All errors (invalid status bytes, oversized snappy responses, corrupt data, etc.) + // are re-thrown so the caller can penalize the peer via handleResponseError. this.logger.debug(`Reading message failed: ${e.message}`); - - let status = ReqRespStatus.UNKNOWN; - if (e instanceof ReqRespStatusError) { - status = e.status; - } - - return { - status, - }; + throw e; } } @@ -780,6 +774,20 @@ export class ReqResp implements ReqRespInterface { return undefined; } + // Invalid status byte: the peer sent a status byte that doesn't match any known status code. + // This is a protocol violation, penalize harshly. + if (e instanceof ReqRespStatusError) { + this.logger.warn(`Invalid status byte from peer ${peerId.toString()} in ${subProtocol}: ${e.message}`, logTags); + return PeerErrorSeverity.LowToleranceError; + } + + // Oversized snappy response: the peer is sending data that exceeds the allowed size. + // This is a protocol violation that wastes bandwidth, so penalize harshly. + if (e instanceof OversizedSnappyResponseError) { + this.logger.warn(`Oversized response from peer ${peerId.toString()} in ${subProtocol}: ${e.message}`, logTags); + return PeerErrorSeverity.LowToleranceError; + } + return this.categorizeConnectionErrors(e, peerId, subProtocol); }