`BeaconBlocksByRoot` accepts unsolicited block roots and skips bad-response peer scoring

[Alleysira]

Describe the bug

Hello developers,

Summary

Prysm's BeaconBlocksByRoot response handling does not check that returned blocks match the requested roots, and the caller does not downscore the peer for a mismatch.

Root cause

prysm/beacon-chain/sync/rpc_send_request.go

Lines 187 to 231 in e86f428

	// SendBeaconBlocksByRootRequest sends BeaconBlocksByRoot and returns fetched blocks, if any.
	func SendBeaconBlocksByRootRequest(
	ctx context.Context, clock blockchain.TemporalOracle, p2pProvider p2p.P2P, pid peer.ID,
	req *p2ptypes.BeaconBlockByRootsReq, blockProcessor BeaconBlockProcessor,
	) ([]interfaces.ReadOnlySignedBeaconBlock, error) {
	topic, err := p2p.TopicFromMessage(p2p.BeaconBlocksByRootsMessageName, slots.ToEpoch(clock.CurrentSlot()))
	if err != nil {
	return nil, err
	}
	stream, err := p2pProvider.Send(ctx, req, topic, pid)
	if err != nil {
	return nil, err
	}
	defer closeStream(stream, log)
	// Augment block processing function, if non-nil block processor is provided.
	blocks := make([]interfaces.ReadOnlySignedBeaconBlock, 0, len(*req))
	process := func(block interfaces.ReadOnlySignedBeaconBlock) error {
	blocks = append(blocks, block)
	if blockProcessor != nil {
	return blockProcessor(block)
	}
	return nil
	}
	currentEpoch := slots.ToEpoch(clock.CurrentSlot())
	for i := 0; i < len(*req); i++ {
	// Exit if peer sends more than max request blocks.
	if uint64(i) >= params.MaxRequestBlock(currentEpoch) {
	break
	}
	isFirstChunk := i == 0
	blk, err := ReadChunkedBlock(stream, clock, p2pProvider, isFirstChunk)
	if errors.Is(err, io.EOF) {
	break
	}
	if err != nil {
	return nil, err
	}
	if err := process(blk); err != nil {
	return nil, err
	}
	}
	return blocks, nil
	}

SendBeaconBlocksByRootRequest appends every decoded block bounded only by len(req) and MaxRequestBlock. It does not compare blk.Block().HashTreeRoot() to the requested roots. The sibling helpers in the same file do validate roots for blobs (blobValidatorFromRootReq), data columns (isSidecarIndexRootRequested), and payload envelopes, only blocks-by-root is missing the check.

Two callers:

• Initial-sync (blocks_fetcher.go:483) passes a nil processor, so findAncestor (blocks_fetcher_utils.go:329) appends blocks[0] even when its root was never requested and walks that block's parent chain.
• Pending-block (rpc_beacon_blocks_by_root.go:47-49) does check the root, but returns a plain fmt.Errorf("received unexpected block with root ...") that does not wrap ErrInvalidFetchedData. sendBatchRootRequest (pending_blocks_queue.go:436-439) logs it at Debug and retries, no BadResponsesScorer.Increment().

Impact

A misbehaving peer can return wrong blocks to block-by-root recovery without being downscored or disconnected, stalling pending-block and backtracking recovery. Worst in low-peer conditions.

Expected:
SendBeaconBlocksByRootRequest should reject blocks whose root was not requested (and over-limit/duplicate responses) so the normal bad-response scoring path penalizes the peer, matching the blob/data-column/envelope helpers

Thanks for your attention!

Has this worked before in a previous version?

N/A

🔬 Minimal Reproduction

Reproduction:

1. Start a devnet with a Prysm node.
2. Connect a peer that completes the Status handshake advertising a head_slot a couple of slots ahead of Prysm, so Prysm selects it for sync.
3. When Prysm sends a BeaconBlocksByRoot request, respond with any valid block whose root was not requested (e.g. a real block from a different slot), for every request.
4. Read the peer's BadResponsesScorer count via the gRPC Debug.GetPeer endpoint before and after.

Observed (v7.1.3, 3 min): 16 wrong-root responses, fault_count stayed at 0 (threshold maxBadResponses=5 not approached), peer connected 140s and repeatedly re-selected for sync.

Error

Platform(s)

Mac (Apple Silicon)

What version of Prysm are you running? (Which release)

v7.1.3

Anything else relevant (validator index / public key)?

No response