[SEC] Unresolved attestations are not checked for valid signatures

[montyly]

Difficulty: Low Severity: High

Description

A lack of signatures validation on unresolved attestations allows an attacker to send malicious attestations.

When an attestation is received, it is checked by isValidAttestation:

https://github.com/status-im/nim-beacon-chain/blob/a84a8ba192e2f4c7bae22470b44b2db12f95ab0f/beacon_chain/attestation_aggregation.nim#L93

If the related block is not yet known, the signature is saved in the pool.unresolved: https://github.com/status-im/nim-beacon-chain/blob/a84a8ba192e2f4c7bae22470b44b2db12f95ab0f/beacon_chain/attestation_aggregation.nim#L158-L163

https://github.com/status-im/nim-beacon-chain/blob/a84a8ba192e2f4c7bae22470b44b2db12f95ab0f/beacon_chain/attestation_pool.nim#L81-L85

The check for its signature in isValidAttestation is done after checking if its related block exists:

https://github.com/status-im/nim-beacon-chain/blob/a84a8ba192e2f4c7bae22470b44b2db12f95ab0f/beacon_chain/attestation_aggregation.nim#L202-L207

However, the signature validation is not done once the block related to an unresolved attestation is found:

https://github.com/status-im/nim-beacon-chain/blob/a84a8ba192e2f4c7bae22470b44b2db12f95ab0f/beacon_chain/attestation_pool.nim#L367-L380

As a result, an attacker can send attestations with incorrect signatures that will be processed by the nbc nodes if they are received before their related blocks.

Exploit Scenario

Eve sends malicious attestations to all the nbc nodes. As a result, all the nbc nodes follow an incorrect fork choice and continuously lose their bets.

Recommendation

Short term, check for the signatures of unresolved attestations once they are resolved.

Long term, thoroughly document every queued mechanism and take into consideration the impact of malicious actors on the queued attestations.

[tersec]

1568 removes the unresolved attestation queue entirely.

[montyly]

Fixed https://github.com/status-im/nimbus-eth2/pull/1568