P2P: beacon_block topic restrictions seem too loose

[keks]

From the spec:

• beacon_block - This topic is used solely for propagating new signed beacon blocks to all nodes on the networks. Signed blocks are sent in their entirety. The following validations MUST pass before forwarding the signed_beacon_block on the network
  • The proposer signature, signed_beacon_block.signature is valid.
  • The block is not from a future slot (with a MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance) -- i.e. validate that signed_beacon_block.message.slot <= current_slot (a client MAY queue future blocks for processing at the appropriate slot).

These rules don't specify not to forward old blocks (i.e. blocks with block.message.slot < current_slot). This looks like a DoS vector.

[dylanlott]

In the aggregate attestation section, one of the validations to check is The aggregate attestation defined by hash_tree_root(aggregate_and_proof.aggregate) has not already been seen (via aggregate gossip, within a block, or through the creation of an equivalent aggregate locally). but this would only check within the last block. Older than one block and it seems the protocol would allow that.

Clarification from team: within means /inside/ the block that's being validated for propagation.

[dylanlott]

Per the spec, nodes keep a cache of of the hash tree roots of aggregate attestation that they've seen within the last ATTESTATION_PROPAGATION_SLOT_RANGE. Given A = hash_tree_root(aap.aggregrate) A must not be seen in this cache to be propagated.

[dylanlott]

Removing lead label since team has confirmed this is a possible vector.

[djrtwo]

Yeah, lack of lower bound on the slot for block propagation is a possible vector.

Even if we just had no-repeat propagation (by hash_tree_root(block)) and no-duplicate propagation (by proposer/slot), having no lower bound on block slot here would require the requisite caches to grow unbounded.

I'm pro the lower bound. Oversight on our part