fix: add reqeust id validation before the buffer allocation

Author: healthykim

eth/protocols/eth/handlers.go

@@ -531,10 +531,7 @@ func handleReceipts70(backend Backend, msg Decoder, peer *Peer) error {
 		return err
 	}
 	if res.LastBlockIncomplete {
-		if err := peer.RequestPartialReceipts(res.RequestId); err != nil {
-			return err
-		}
-		return nil
+		return peer.RequestPartialReceipts(res.RequestId)
 	}
 
 	// Assign buffers shared between list elements

eth/protocols/eth/peer.go

@@ -357,6 +357,7 @@ func (p *Peer) RequestReceipts(hashes []common.Hash, sink chan *Response) (*Requ
 				FirstBlockReceiptIndex: 0,
 			},
 		}
+		p.requestedReceipts[id] = hashes
 	} else {
 		req = &Request{
 			id:   id,
@@ -373,7 +374,6 @@ func (p *Peer) RequestReceipts(hashes []common.Hash, sink chan *Response) (*Requ
 		return nil, err
 	}
 
-	p.requestedReceipts[id] = hashes
 	return req, nil
 }
 
@@ -406,6 +406,12 @@ func (p *Peer) RequestPartialReceipts(id uint64) error {
 // If the request is completed, it appends previously collected receipts.
 func (p *Peer) BufferReceiptsPacket(packet *ReceiptsPacket70) error {
 	requestId := packet.RequestId
+
+	// Do not assign buffer to the response not requested
+	if _, ok := p.requestedReceipts[requestId]; !ok {
+		return fmt.Errorf("No partial receipt retreival in progress with id %d", requestId)
+	}
+
 	if len(packet.List) == 0 {
 		return nil
 	}

eth/protocols/eth/peer_test.go

@@ -211,3 +211,55 @@ func TestPartialReceipt(t *testing.T) {
 		t.Fatalf("requestedReceipts should be cleared after delivery")
 	}
 }
+
+func TestPartialReceiptDisconnectRequestId(t *testing.T) {
+	app, net := p2p.MsgPipe()
+	var id enode.ID
+	if _, err := rand.Read(id[:]); err != nil {
+		t.Fatalf("failed to create random peer: %v", err)
+	}
+
+	peer := NewPeer(ETH70, p2p.NewPeer(id, "peer", nil), net, nil)
+
+	packetCh := make(chan *GetReceiptsPacket70, 1)
+	go func() {
+		for {
+			msg, err := app.ReadMsg()
+			if err != nil {
+				return
+			}
+			if msg.Code == GetReceiptsMsg {
+				var pkt GetReceiptsPacket70
+				if err := msg.Decode(&pkt); err == nil {
+					select {
+					case packetCh <- &pkt:
+					default:
+					}
+				}
+			}
+			msg.Discard()
+		}
+	}()
+
+	// If a peer delivers response which is never requested, it should fail the validation
+	delivery := &ReceiptsPacket70{
+		RequestId:           66,
+		LastBlockIncomplete: true,
+		List: []*ReceiptList69{
+			{
+				items: []Receipt{
+					{GasUsed: 21_000, Logs: rlp.RawValue(make([]byte, 1))},
+				},
+			},
+			{
+				items: []Receipt{
+					{GasUsed: 21_000, Logs: rlp.RawValue(make([]byte, 2))},
+				},
+			},
+		},
+	}
+	err := peer.BufferReceiptsPacket(delivery)
+	if err == nil {
+		t.Fatal("Unknown response should be dropped")
+	}
+}

eth/protocols/eth/receipt.go

@@ -403,6 +403,11 @@ func (rl *ReceiptList69) EncodeRLP(_w io.Writer) error {
 	return w.Flush()
 }
 
+// Append appends all items from another ReceiptList69 to this list.
+func (rl *ReceiptList69) Append(other *ReceiptList69) {
+	rl.items = append(rl.items, other.items...)
+}
+
 // blockReceiptsToNetwork69 takes a slice of rlp-encoded receipts, and transactions,
 // and applies the type-encoding on the receipts (for non-legacy receipts).
 // e.g. for non-legacy receipts: receipt-data -> {tx-type || receipt-data}