From 9b364aa7eed5845dba65d03e86b79d90ecfc495c Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 15 Dec 2022 20:10:09 +0300 Subject: [PATCH] network: do not allow to request invalid block count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The problem is in peer disconnection due to invalid GetBlockByIndex payload (the logs are from some patched neo-go version): ``` дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.490Z INFO new peer connected {"addr": "10.78.69.115:50846", "peerCount": 3} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.490Z WARN peer disconnected {"addr": "10.78.69.115:50846", "error": "invalid block count", "peerCount": 2} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.490Z INFO started protocol {"addr": "10.78.69.115:50846", "userAgent": "/NEO-GO:1.0.0/", "startHeight": 0, "id": 1339571820} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.491Z INFO new peer connected {"addr": "10.78.69.115:50856", "peerCount": 3} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.492Z WARN peer disconnected {"addr": "10.78.69.115:50856", "error": "invalid block count", "peerCount": 2} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.492Z INFO started protocol {"addr": "10.78.69.115:50856", "userAgent": "/NEO-GO:1.0.0/", "startHeight": 0, "id": 1339571820} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.492Z INFO new peer connected {"addr": "10.78.69.115:50858", "peerCount": 3} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.493Z INFO started protocol {"addr": "10.78.69.115:50858", "userAgent": "/NEO-GO:1.0.0/", "startHeight": 0, "id": 1339571820} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.493Z WARN peer disconnected {"addr": "10.78.69.115:50858", "error": "invalid block count", "peerCount": 2} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.494Z INFO new peer connected {"addr": "10.78.69.115:50874", "peerCount": 3} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.494Z INFO started protocol {"addr": "10.78.69.115:50874", "userAgent": "/NEO-GO:1.0.0/", "startHeight": 0, "id": 1339571820} дек 15 16:02:39 glagoli neo-go[928530]: 2022-12-15T16:02:39.494Z WARN peer disconnected {"addr": "10.78.69.115:50874", "error": "invalid block count", "peerCount": 2} ``` GetBlockByIndex payload can't be decoded, and the only possible cause is zero (or <-1, but it's probably not the case) block count requested. Error is improved as far. --- pkg/network/blockqueue.go | 6 ++++-- pkg/network/blockqueue_test.go | 20 +++++++++++++++----- pkg/network/payload/getblockbyindex.go | 4 ++-- pkg/network/server.go | 14 ++++++++------ 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/pkg/network/blockqueue.go b/pkg/network/blockqueue.go index 54de16106..5927b72f7 100644 --- a/pkg/network/blockqueue.go +++ b/pkg/network/blockqueue.go @@ -136,10 +136,12 @@ func (bq *blockQueue) putBlock(block *block.Block) error { return nil } -func (bq *blockQueue) lastQueued() uint32 { +// lastQueued returns the index of the last queued block and the queue's capacity +// left. +func (bq *blockQueue) lastQueued() (uint32, int) { bq.queueLock.RLock() defer bq.queueLock.RUnlock() - return bq.lastQ + return bq.lastQ, blockCacheSize - bq.len } func (bq *blockQueue) discard() { diff --git a/pkg/network/blockqueue_test.go b/pkg/network/blockqueue_test.go index f647ebd0f..642ba8ccd 100644 --- a/pkg/network/blockqueue_test.go +++ b/pkg/network/blockqueue_test.go @@ -22,7 +22,9 @@ func TestBlockQueue(t *testing.T) { for i := 3; i < 5; i++ { assert.NoError(t, bq.putBlock(blocks[i])) } - assert.Equal(t, uint32(0), bq.lastQueued()) + last, capLeft := bq.lastQueued() + assert.Equal(t, uint32(0), last) + assert.Equal(t, blockCacheSize-2, capLeft) // nothing should be put into the blockchain assert.Equal(t, uint32(0), chain.BlockHeight()) assert.Equal(t, 2, bq.length()) @@ -31,7 +33,9 @@ func TestBlockQueue(t *testing.T) { assert.NoError(t, bq.putBlock(blocks[i])) } // but they're still not put into the blockchain, because bq isn't running - assert.Equal(t, uint32(4), bq.lastQueued()) + last, capLeft = bq.lastQueued() + assert.Equal(t, uint32(4), last) + assert.Equal(t, blockCacheSize-4, capLeft) assert.Equal(t, uint32(0), chain.BlockHeight()) assert.Equal(t, 4, bq.length()) // block with too big index is dropped @@ -40,14 +44,18 @@ func TestBlockQueue(t *testing.T) { go bq.run() // run() is asynchronous, so we need some kind of timeout anyway and this is the simplest one assert.Eventually(t, func() bool { return chain.BlockHeight() == 4 }, 4*time.Second, 100*time.Millisecond) - assert.Equal(t, uint32(4), bq.lastQueued()) + last, capLeft = bq.lastQueued() + assert.Equal(t, uint32(4), last) + assert.Equal(t, blockCacheSize, capLeft) assert.Equal(t, 0, bq.length()) assert.Equal(t, uint32(4), chain.BlockHeight()) // put some old blocks for i := 1; i < 5; i++ { assert.NoError(t, bq.putBlock(blocks[i])) } - assert.Equal(t, uint32(4), bq.lastQueued()) + last, capLeft = bq.lastQueued() + assert.Equal(t, uint32(4), last) + assert.Equal(t, blockCacheSize, capLeft) assert.Equal(t, 0, bq.length()) assert.Equal(t, uint32(4), chain.BlockHeight()) // unexpected blocks with run() active @@ -65,7 +73,9 @@ func TestBlockQueue(t *testing.T) { assert.NoError(t, bq.putBlock(blocks[5])) // run() is asynchronous, so we need some kind of timeout anyway and this is the simplest one assert.Eventually(t, func() bool { return chain.BlockHeight() == 8 }, 4*time.Second, 100*time.Millisecond) - assert.Equal(t, uint32(8), bq.lastQueued()) + last, capLeft = bq.lastQueued() + assert.Equal(t, uint32(8), last) + assert.Equal(t, blockCacheSize-1, capLeft) assert.Equal(t, 1, bq.length()) assert.Equal(t, uint32(8), chain.BlockHeight()) bq.discard() diff --git a/pkg/network/payload/getblockbyindex.go b/pkg/network/payload/getblockbyindex.go index 24f573014..e25f62e4b 100644 --- a/pkg/network/payload/getblockbyindex.go +++ b/pkg/network/payload/getblockbyindex.go @@ -1,7 +1,7 @@ package payload import ( - "errors" + "fmt" "github.com/nspcc-dev/neo-go/pkg/io" ) @@ -25,7 +25,7 @@ func (d *GetBlockByIndex) DecodeBinary(br *io.BinReader) { d.IndexStart = br.ReadU32LE() d.Count = int16(br.ReadU16LE()) if d.Count < -1 || d.Count == 0 || d.Count > MaxHeadersAllowed { - br.Err = errors.New("invalid block count") + br.Err = fmt.Errorf("invalid block count: %d", d.Count) } } diff --git a/pkg/network/server.go b/pkg/network/server.go index 898d4246f..9f5572e77 100644 --- a/pkg/network/server.go +++ b/pkg/network/server.go @@ -1247,13 +1247,15 @@ func (s *Server) handleGetAddrCmd(p Peer) error { // 2. Send requests for chunk in increasing order. // 3. After all requests have been sent, request random height. func (s *Server) requestBlocks(bq Blockqueuer, p Peer) error { - h := bq.BlockHeight() - pl := getRequestBlocksPayload(p, h, &s.lastRequestedBlock) - lq := s.bQueue.lastQueued() + pl := getRequestBlocksPayload(p, bq.BlockHeight(), &s.lastRequestedBlock) + lq, capLeft := s.bQueue.lastQueued() + if capLeft == 0 { + // No more blocks will fit into the queue. + return nil + } if lq >= pl.IndexStart { - c := int16(h + blockCacheSize - lq) - if c < payload.MaxHashesCount { - pl.Count = c + if capLeft < payload.MaxHashesCount { + pl.Count = int16(capLeft) } pl.IndexStart = lq + 1 }