From 3a5224344ed6bc27f8665f71f96ab907f19e6874 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Sat, 29 Feb 2020 17:52:09 +0300 Subject: [PATCH] core: verify headers in AddHeaders() Headers can be malformed so public methods should verify them before adding. --- pkg/core/blockchain.go | 64 +++++++++++++++++++++++-------------- pkg/core/blockchain_test.go | 25 +++++++++++---- 2 files changed, 59 insertions(+), 30 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 32361f61f..6de1a35e8 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -301,11 +301,16 @@ func (bc *Blockchain) AddBlock(block *block.Block) error { if expectedHeight != block.Index { return fmt.Errorf("expected block %d, but passed block %d", expectedHeight, block.Index) } + + headerLen := bc.headerListLen() + if int(block.Index) == headerLen { + err := bc.addHeaders(bc.config.VerifyBlocks, block.Header()) + if err != nil { + return err + } + } if bc.config.VerifyBlocks { err := block.Verify() - if err == nil { - err = bc.VerifyBlock(block) - } if err != nil { return fmt.Errorf("block %s is invalid: %s", block.Hash().StringLE(), err) } @@ -318,24 +323,37 @@ func (bc *Blockchain) AddBlock(block *block.Block) error { } } } - headerLen := bc.headerListLen() - if int(block.Index) == headerLen { - err := bc.AddHeaders(block.Header()) - if err != nil { - return err - } - } return bc.storeBlock(block) } // AddHeaders processes the given headers and add them to the // HeaderHashList. -func (bc *Blockchain) AddHeaders(headers ...*block.Header) (err error) { +func (bc *Blockchain) AddHeaders(headers ...*block.Header) error { + return bc.addHeaders(bc.config.VerifyBlocks, headers...) +} + +func (bc *Blockchain) addHeaders(verify bool, headers ...*block.Header) (err error) { var ( start = time.Now() batch = bc.dao.store.Batch() ) + if len(headers) == 0 { + return nil + } else if verify { + // Verify that the chain of the headers is consistent. + var lastHeader *block.Header + if lastHeader, err = bc.GetHeader(headers[0].PrevHash); err != nil { + return fmt.Errorf("previous header was not found: %v", err) + } + for _, h := range headers { + if err = bc.verifyHeader(h, lastHeader); err != nil { + return + } + lastHeader = h + } + } + bc.headersOp <- func(headerList *HeaderHashList) { oldlen := headerList.Len() for _, h := range headers { @@ -1101,19 +1119,17 @@ func (bc *Blockchain) ApplyPolicyToTxSet(txes []mempool.TxWithFee) []mempool.TxW return txes } -// VerifyBlock verifies block against its current state. -func (bc *Blockchain) VerifyBlock(block *block.Block) error { - prevHeader, err := bc.GetHeader(block.PrevHash) - if err != nil { - return errors.Wrap(err, "unable to get previous header") +func (bc *Blockchain) verifyHeader(currHeader, prevHeader *block.Header) error { + if prevHeader.Hash() != currHeader.PrevHash { + return errors.New("previous header hash doesn't match") } - if prevHeader.Index+1 != block.Index { + if prevHeader.Index+1 != currHeader.Index { return errors.New("previous header index doesn't match") } - if prevHeader.Timestamp >= block.Timestamp { + if prevHeader.Timestamp >= currHeader.Timestamp { return errors.New("block is not newer than the previous one") } - return bc.verifyBlockWitnesses(block, prevHeader) + return bc.verifyHeaderWitnesses(currHeader, prevHeader) } // verifyTx verifies whether a transaction is bonafide or not. @@ -1678,16 +1694,16 @@ func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *block return nil } -// verifyBlockWitnesses is a block-specific implementation of VerifyWitnesses logic. -func (bc *Blockchain) verifyBlockWitnesses(block *block.Block, prevHeader *block.Header) error { +// verifyHeaderWitnesses is a block-specific implementation of VerifyWitnesses logic. +func (bc *Blockchain) verifyHeaderWitnesses(currHeader, prevHeader *block.Header) error { var hash util.Uint160 - if prevHeader == nil && block.PrevHash.Equals(util.Uint256{}) { - hash = block.Script.ScriptHash() + if prevHeader == nil && currHeader.PrevHash.Equals(util.Uint256{}) { + hash = currHeader.Script.ScriptHash() } else { hash = prevHeader.NextConsensus } interopCtx := bc.newInteropContext(trigger.Verification, bc.dao.store, nil, nil) - return bc.verifyHashAgainstScript(hash, &block.Script, block.VerificationHash(), interopCtx, true) + return bc.verifyHashAgainstScript(hash, &currHeader.Script, currHeader.VerificationHash(), interopCtx, true) } func hashAndIndexToBytes(h util.Uint256, index uint32) []byte { diff --git a/pkg/core/blockchain_test.go b/pkg/core/blockchain_test.go index 2009dd978..88fe525d4 100644 --- a/pkg/core/blockchain_test.go +++ b/pkg/core/blockchain_test.go @@ -20,22 +20,35 @@ func TestAddHeaders(t *testing.T) { h2 := newBlock(bc.config, 2, h1.Hash()).Header() h3 := newBlock(bc.config, 3, h2.Hash()).Header() - if err := bc.AddHeaders(h1, h2, h3); err != nil { - t.Fatal(err) - } + require.NoError(t, bc.AddHeaders()) + require.NoError(t, bc.AddHeaders(h1, h2)) + require.NoError(t, bc.AddHeaders(h2, h3)) assert.Equal(t, h3.Index, bc.HeaderHeight()) assert.Equal(t, uint32(0), bc.BlockHeight()) assert.Equal(t, h3.Hash(), bc.CurrentHeaderHash()) // Add them again, they should not be added. - if err := bc.AddHeaders(h3, h2, h1); err != nil { - t.Fatal(err) - } + require.Error(t, bc.AddHeaders(h3, h2, h1)) assert.Equal(t, h3.Index, bc.HeaderHeight()) assert.Equal(t, uint32(0), bc.BlockHeight()) assert.Equal(t, h3.Hash(), bc.CurrentHeaderHash()) + + h4 := newBlock(bc.config, 4, h3.Hash().Reverse()).Header() + h5 := newBlock(bc.config, 5, h4.Hash()).Header() + + assert.Error(t, bc.AddHeaders(h4, h5)) + assert.Equal(t, h3.Index, bc.HeaderHeight()) + assert.Equal(t, uint32(0), bc.BlockHeight()) + assert.Equal(t, h3.Hash(), bc.CurrentHeaderHash()) + + h6 := newBlock(bc.config, 4, h3.Hash()).Header() + h6.Script.InvocationScript = nil + assert.Error(t, bc.AddHeaders(h6)) + assert.Equal(t, h3.Index, bc.HeaderHeight()) + assert.Equal(t, uint32(0), bc.BlockHeight()) + assert.Equal(t, h3.Hash(), bc.CurrentHeaderHash()) } func TestAddBlock(t *testing.T) {