Verify IBFT preprepare proposal block number matches sequence number (#…

…604)
celo-org · Nov 20, 2019 · 6ff30e7 · 6ff30e7
1 parent 39695bb
commit 6ff30e7
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 9 deletions.
diff --git a/consensus/istanbul/core/preprepare.go b/consensus/istanbul/core/preprepare.go
@@ -109,6 +109,12 @@ func (c *core) handlePreprepare(msg *istanbul.Message) error {
 		return errNotFromProposer
 	}
 
+	// Verify that the proposal is for the sequence number of the view we verified.
+	if preprepare.View.Sequence.Cmp(preprepare.Proposal.Number()) != 0 {
+		logger.Warn("Received preprepare with invalid block number", "number", preprepare.Proposal.Number(), "view_seq", preprepare.View.Sequence)
+		return errInvalidProposal
+	}
+
 	// Verify the proposal we received
 	if duration, err := c.backend.Verify(preprepare.Proposal); err != nil {
 		logger.Warn("Failed to verify proposal", "err", err, "duration", duration)

diff --git a/consensus/istanbul/core/preprepare_test.go b/consensus/istanbul/core/preprepare_test.go
@@ -63,6 +63,27 @@ func TestHandlePreprepare(t *testing.T) {
 			nil,
 			false,
 		},
+		{
+			// proposal sequence doesn't match message sequence
+			func() *testSystem {
+				sys := NewTestSystemWithBackend(N, F)
+
+				for i, backend := range sys.backends {
+					c := backend.engine.(*core)
+					c.valSet = backend.peers
+					if i != 0 {
+						c.state = StateAcceptRequest
+					}
+				}
+				return sys
+			}(),
+			func(_ *testSystem) istanbul.RoundChangeCertificate {
+				return istanbul.RoundChangeCertificate{}
+			},
+			makeBlock(3),
+			errInvalidProposal,
+			false,
+		},
 		{
 			// non-proposer
 			func() *testSystem {
@@ -116,14 +137,12 @@ func TestHandlePreprepare(t *testing.T) {
 			func() *testSystem {
 				sys := NewTestSystemWithBackend(N, F)
 
-				for i, backend := range sys.backends {
+				for _, backend := range sys.backends {
 					c := backend.engine.(*core)
 					c.valSet = backend.peers
-					if i != 0 {
-						c.state = StatePreprepared
-						c.current.SetSequence(big.NewInt(10))
-						c.current.SetRound(big.NewInt(10))
-					}
+					c.state = StatePreprepared
+					c.current.SetSequence(big.NewInt(10))
+					c.current.SetRound(big.NewInt(10))
 				}
 				return sys
 			}(),
@@ -244,7 +263,6 @@ func TestHandlePreprepare(t *testing.T) {
 			errInvalidPreparedCertificateDigestMismatch,
 			false,
 		},
-
 		{
 			// ROUND CHANGE certificate for N+1 round with valid PREPARED certificates
 			// Round is N+1 to match the correct proposer.
@@ -262,11 +280,11 @@ func TestHandlePreprepare(t *testing.T) {
 				return sys
 			}(),
 			func(sys *testSystem) istanbul.RoundChangeCertificate {
-				preparedCertificate := sys.getPreparedCertificate(t, []istanbul.View{*(sys.backends[0].engine.(*core).current.View())}, makeBlock(0))
+				preparedCertificate := sys.getPreparedCertificate(t, []istanbul.View{*(sys.backends[0].engine.(*core).current.View())}, makeBlock(1))
 				roundChangeCertificate := sys.getRoundChangeCertificate(t, *(sys.backends[0].engine.(*core).current.View()), preparedCertificate)
 				return roundChangeCertificate
 			},
-			makeBlock(0),
+			makeBlock(1),
 			nil,
 			false,
 		},