status-im / nimbus-eth2

Nim implementation of the Ethereum Beacon Chain
https://nimbus.guide
Other
543 stars 233 forks source link

[SEC] Attackers can corrupt quarantined blocks #1552

Closed montyly closed 3 years ago

montyly commented 4 years ago

Severity: Low Difficulty: Low

Description

A key mapping collision allows attackers to replace legitimate quarantined blocks with malicious block

When a block is received, it is checked with clearance.isValidBeaconBlock: https://github.com/status-im/nim-beacon-chain/blob/3f9b408c65c03f52bfc755b35191685b5e8005f6/beacon_chain/block_pools/clearance.nim#L257-L260

If the parent of the block is unknown, the block will be queued in quarantine: https://github.com/status-im/nim-beacon-chain/blob/3f9b408c65c03f52bfc755b35191685b5e8005f6/beacon_chain/block_pools/clearance.nim#L333-L341

quarantine.add stores the block in a mapping, where block.root is the key https://github.com/status-im/nim-beacon-chain/blob/3f9b408c65c03f52bfc755b35191685b5e8005f6/beacon_chain/block_pools/quarantine.nim#L46-L49

block.root is the hash for the message, and does not include the signature: https://github.com/status-im/nim-beacon-chain/blob/3433c77c35f3025729ac9204ab6b91ea136dce9e/beacon_chain/ssz/bytes_reader.nim#L271

If two blocks with the same message, but with a different signature, are quanrantined, only the latest one will be saved. As a result, an attacker can delete quarantined block by sending a malicious block with an incorrect signature

Exploit Scenario

Bob's node receive a message from Alice's node, for which he does not have the parent. The block is saved in quarantine. Eve sends to Bob a block with the same message, but an incorrect signature. As a result Alice's block is deleted, and Bob never receives the legitimate signature.

Mitigation Recommendation

Short term, review every usage of block.root and consider to either:

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

tersec commented 4 years ago

https://github.com/status-im/nim-beacon-chain/pull/1601 fixes this.

montyly commented 3 years ago

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