search

onflow / flow-go

A fast, secure, and developer-friendly blockchain built to support the next generation of games, apps, and the digital assets that power them.
GNU Affero General Public License v3.0
533 stars 179 forks source link

[Malleability C] hotstuff CertifiedBlock #6718

Open UlyanaAndrukhiv opened 2 weeks ago

UlyanaAndrukhiv commented 2 weeks ago

Problem description

CertifiedBlock is a Block with an acquired quorum. ID() implementation is wrong as it doesn't cover the quorum certificate, a byzantine consensus node can change the QC without being noticed by other nodes. The usage of this data structure should be reviewed to be proven safe.

https://github.com/onflow/flow-go/blob/edf27b03b6f809ce66cacd607a41b889c12cd2b3/consensus/hotstuff/model/block.go#L10-L19

https://github.com/onflow/flow-go/blob/edf27b03b6f809ce66cacd607a41b889c12cd2b3/consensus/hotstuff/model/block.go#L49-L56

https://github.com/onflow/flow-go/blob/edf27b03b6f809ce66cacd607a41b889c12cd2b3/consensus/hotstuff/model/block.go#L72-L76

https://github.com/onflow/flow-go/blob/edf27b03b6f809ce66cacd607a41b889c12cd2b3/model/flow/quorum_certificate.go#L3-L23

Proposed solution

Change implementation of ID() so that it hashes the quorum certificate too