eth/downloader: prevent a peer from dripping bad hashes

2 files changed, 36 insertions, 9 deletions

diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go
index d817b223c..f3a866441 100644
--- a/eth/downloader/downloader.go
+++ b/eth/downloader/downloader.go
@@ -28,10 +28,11 @@ var (
 )
 
 var (
-	errLowTd            = errors.New("peer's TD is too low")
+	errLowTd            = errors.New("peers TD is too low")
 	ErrBusy             = errors.New("busy")
-	errUnknownPeer      = errors.New("peer's unknown or unhealthy")
+	errUnknownPeer      = errors.New("peer is unknown or unhealthy")
 	ErrBadPeer          = errors.New("action from bad peer ignored")
+	ErrStallingPeer     = errors.New("peer is stalling")
 	errNoPeers          = errors.New("no peers to keep download active")
 	ErrPendingQueue     = errors.New("pending items in queue")
 	ErrTimeout          = errors.New("timeout")
@@ -283,15 +284,18 @@ func (d *Downloader) fetchHashes(p *peer, h common.Hash) error {
 				return ErrBadPeer
 			}
 			if !done {
+				// Check that the peer is not stalling the sync
+				if len(inserts) < maxHashFetch {
+					return ErrStallingPeer
+				}
 				// Try and fetch a random block to verify the hash batch
 				// Skip the last hash as the cross check races with the next hash fetch
-				if len(inserts) > 1 {
-					cross := inserts[rand.Intn(len(inserts)-1)]
-					glog.V(logger.Detail).Infof("Cross checking (%s) with %x", active.id, cross)
+				cross := inserts[rand.Intn(len(inserts)-1)]
+				glog.V(logger.Detail).Infof("Cross checking (%s) with %x", active.id, cross)
+
+				d.checks[cross] = time.Now().Add(blockTTL)
+				active.getBlocks([]common.Hash{cross})
 
-					d.checks[cross] = time.Now().Add(blockTTL)
-					active.getBlocks([]common.Hash{cross})
-				}
 				// Also fetch a fresh
 				active.getHashes(head)
 				continue
diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go
index 19d64ac67..8ed3289c6 100644
--- a/eth/downloader/downloader_test.go
+++ b/eth/downloader/downloader_test.go
@@ -53,6 +53,8 @@ type downloadTester struct {
 	blocks map[common.Hash]*types.Block // Blocks associated with the hashes
 	chain  []common.Hash                // Block-chain being constructed
 
+	maxHashFetch int // Overrides the maximum number of retrieved hashes
+
 	t            *testing.T
 	pcount       int
 	done         chan bool
@@ -133,8 +135,12 @@ func (dl *downloadTester) getBlock(hash common.Hash) *types.Block {
 
 // getHashes retrieves a batch of hashes for reconstructing the chain.
 func (dl *downloadTester) getHashes(head common.Hash) error {
+	limit := maxHashFetch
+	if dl.maxHashFetch > 0 {
+		limit = dl.maxHashFetch
+	}
 	// Gather the next batch of hashes
-	hashes := make([]common.Hash, 0, maxHashFetch)
+	hashes := make([]common.Hash, 0, limit)
 	for i, hash := range dl.hashes {
 		if hash == head {
 			i++
@@ -469,6 +475,23 @@ func TestMadeupHashChainAttack(t *testing.T) {
 	}
 }
 
+// Tests that if a malicious peer makes up a random hash chain, and tries to push
+// indefinitely, one hash at a time, it actually gets caught with it. The reason
+// this is separate from the classical made up chain attack is that sending hashes
+// one by one prevents reliable block/parent verification.
+func TestMadeupHashChainDrippingAttack(t *testing.T) {
+	// Create a random chain of hashes to drip
+	hashes := createHashes(0, 16*blockCacheLimit)
+	tester := newTester(t, hashes, nil)
+
+	// Try and sync with the attacker, one hash at a time
+	tester.maxHashFetch = 1
+	tester.newPeer("attack", big.NewInt(10000), hashes[0])
+	if _, err := tester.syncTake("attack", hashes[0]); err != ErrStallingPeer {
+		t.Fatalf("synchronisation error mismatch: have %v, want %v", err, ErrStallingPeer)
+	}
+}
+
 // Tests that if a malicious peer makes up a random block chain, and tried to
 // push indefinitely, it actually gets caught with it.
 func TestMadeupBlockChainAttack(t *testing.T) {