From c492b009978ef142c826f5940944aacc873ad602 Mon Sep 17 00:00:00 2001 From: Ulrich Petri Date: Thu, 23 Jun 2016 19:38:20 +0200 Subject: [PATCH] Fix critical unintentional variable name reuse In `devp2p.discovery.recv_neighbours()` the variable `node` (ostensibly containing the sender node) was reused as a temporary variable inside a for loop. Depending on the order of the `neighbours_lst` this lead in many cases to neighbour information being attributed to the wrong sender node. Thanks to @konradkonrad for the pair debugging :) Refs: HydraChain/hydrachain#73 --- devp2p/discovery.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/devp2p/discovery.py b/devp2p/discovery.py index 7cee4b5..1124943 100644 --- a/devp2p/discovery.py +++ b/devp2p/discovery.py @@ -489,24 +489,22 @@ def send_neighbours(self, node, neighbours): self.send(node, message) def recv_neighbours(self, nodeid, payload, mdc): - node = self.get_node(nodeid) + remote = self.get_node(nodeid) assert len(payload) == 1 neighbours_lst = payload[0] assert isinstance(neighbours_lst, list) - log.debug('<<< neigbours', remoteid=node, count=len(neighbours_lst)) - neighbours = [] - neighbours_set = set(tuple(x) for x in neighbours_lst) - if len(neighbours_set) < len(neighbours_lst): - log.warn('received duplicates') - for n in neighbours_set: - n = list(n) + neighbours = [] + for n in neighbours_lst: nodeid = n.pop() address = Address.from_endpoint(*n) node = self.get_node(nodeid, address) assert node.address neighbours.append(node) - self.kademlia.recv_neighbours(node, neighbours) + + log.debug('<<< neighbours', remoteid=remote, local=self.this_node, neighbours=neighbours, + count=len(neighbours_lst)) + self.kademlia.recv_neighbours(remote, neighbours) class NodeDiscovery(BaseService, DiscoveryProtocolTransport):