2 files changed, 56 insertions, 13 deletions

diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go
index f3a866441..f0629a551 100644
--- a/eth/downloader/downloader.go
+++ b/eth/downloader/downloader.go
@@ -61,13 +61,18 @@ type hashPack struct {
 	hashes []common.Hash
 }
 
+type crossCheck struct {
+	expire time.Time
+	parent common.Hash
+}
+
 type Downloader struct {
 	mux *event.TypeMux
 
 	mu     sync.RWMutex
-	queue  *queue                    // Scheduler for selecting the hashes to download
-	peers  *peerSet                  // Set of active peers from which download can proceed
-	checks map[common.Hash]time.Time // Pending cross checks to verify a hash chain
+	queue  *queue                      // Scheduler for selecting the hashes to download
+	peers  *peerSet                    // Set of active peers from which download can proceed
+	checks map[common.Hash]*crossCheck // Pending cross checks to verify a hash chain
 
 	// Callbacks
 	hasBlock hashCheckFn
@@ -158,7 +163,7 @@ func (d *Downloader) Synchronise(id string, hash common.Hash) error {
 	// Reset the queue and peer set to clean any internal leftover state
 	d.queue.Reset()
 	d.peers.Reset()
-	d.checks = make(map[common.Hash]time.Time)
+	d.checks = make(map[common.Hash]*crossCheck)
 
 	// Retrieve the origin peer and initiate the downloading process
 	p := d.peers.Peer(id)
@@ -290,11 +295,15 @@ func (d *Downloader) fetchHashes(p *peer, h common.Hash) error {
 				}
 				// 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
-				cross := inserts[rand.Intn(len(inserts)-1)]
-				glog.V(logger.Detail).Infof("Cross checking (%s) with %x", active.id, cross)
+				cross := rand.Intn(len(inserts) - 1)
+				origin, parent := inserts[cross], inserts[cross+1]
+				glog.V(logger.Detail).Infof("Cross checking (%s) with %x/%x", active.id, origin, parent)
 
-				d.checks[cross] = time.Now().Add(blockTTL)
-				active.getBlocks([]common.Hash{cross})
+				d.checks[origin] = &crossCheck{
+					expire: time.Now().Add(blockTTL),
+					parent: parent,
+				}
+				active.getBlocks([]common.Hash{origin})
 
 				// Also fetch a fresh
 				active.getHashes(head)
@@ -314,8 +323,8 @@ func (d *Downloader) fetchHashes(p *peer, h common.Hash) error {
 				continue
 			}
 			block := blockPack.blocks[0]
-			if _, ok := d.checks[block.Hash()]; ok {
-				if !d.queue.Has(block.ParentHash()) {
+			if check, ok := d.checks[block.Hash()]; ok {
+				if block.ParentHash() != check.parent {
 					return ErrCrossCheckFailed
 				}
 				delete(d.checks, block.Hash())
@@ -323,8 +332,8 @@ func (d *Downloader) fetchHashes(p *peer, h common.Hash) error {
 
 		case <-crossTicker.C:
 			// Iterate over all the cross checks and fail the hash chain if they're not verified
-			for hash, deadline := range d.checks {
-				if time.Now().After(deadline) {
+			for hash, check := range d.checks {
+				if time.Now().After(check.expire) {
 					glog.V(logger.Debug).Infof("Cross check timeout for %x", hash)
 					return ErrCrossCheckFailed
 				}
diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go
index 8ed3289c6..d623a7c76 100644
--- a/eth/downloader/downloader_test.go
+++ b/eth/downloader/downloader_test.go
@@ -502,7 +502,7 @@ func TestMadeupBlockChainAttack(t *testing.T) {
 	crossCheckCycle = 25 * time.Millisecond
 
 	// Create a long chain of blocks and simulate an invalid chain by dropping every second
-	hashes := createHashes(0, 32*blockCacheLimit)
+	hashes := createHashes(0, 16*blockCacheLimit)
 	blocks := createBlocksFromHashes(hashes)
 
 	gapped := make([]common.Hash, len(hashes)/2)
@@ -525,3 +525,37 @@ func TestMadeupBlockChainAttack(t *testing.T) {
 		t.Fatalf("failed to synchronise blocks: %v", err)
 	}
 }
+
+// Advanced form of the above forged blockchain attack, where not only does the
+// attacker make up a valid hashes for random blocks, but also forges the block
+// parents to point to existing hashes.
+func TestMadeupParentBlockChainAttack(t *testing.T) {
+	defaultBlockTTL := blockTTL
+	defaultCrossCheckCycle := crossCheckCycle
+
+	blockTTL = 100 * time.Millisecond
+	crossCheckCycle = 25 * time.Millisecond
+
+	// Create a long chain of blocks and simulate an invalid chain by dropping every second
+	hashes := createHashes(0, 16*blockCacheLimit)
+	blocks := createBlocksFromHashes(hashes)
+	forges := createBlocksFromHashes(hashes)
+	for hash, block := range forges {
+		block.ParentHeaderHash = hash // Simulate pointing to already known hash
+	}
+	// Try and sync with the malicious node and check that it fails
+	tester := newTester(t, hashes, forges)
+	tester.newPeer("attack", big.NewInt(10000), hashes[0])
+	if _, err := tester.syncTake("attack", hashes[0]); err != ErrCrossCheckFailed {
+		t.Fatalf("synchronisation error mismatch: have %v, want %v", err, ErrCrossCheckFailed)
+	}
+	// Ensure that a valid chain can still pass sync
+	blockTTL = defaultBlockTTL
+	crossCheckCycle = defaultCrossCheckCycle
+
+	tester.blocks = blocks
+	tester.newPeer("valid", big.NewInt(20000), hashes[0])
+	if _, err := tester.syncTake("valid", hashes[0]); err != nil {
+		t.Fatalf("failed to synchronise blocks: %v", err)
+	}
+}