Add IP checks on ENRs received from Nodes packet

eth/p2p/discoveryv5/node.nim

@@ -38,6 +38,8 @@ proc newNode*(r: Record): Node =
                               address_v4: ipBytes),
                 udpPort: Port udpPort)
   except KeyError:
+    # TODO: This will result in a 0.0.0.0 address. Might introduce more bugs.
+    # Lets fail when creating a node just form a record without IP?
     discard
 
   var pk: PublicKey

eth/p2p/discoveryv5/protocol.nim

@@ -259,6 +259,23 @@ proc processClient(transp: DatagramTransport,
     debug "Receive failed", exception = e.name, msg = e.msg,
       stacktrace = e.getStackTrace()
 
+proc validIp(sender, address: IpAddress): bool =
+  let
+    s = initTAddress(sender, Port(0))
+    a = initTAddress(address, Port(0))
+  if a.isAnyLocal():
+    return false
+  if a.isMulticast():
+    return false
+  if a.isLoopback() and not s.isLoopback():
+    return false
+  if a.isSiteLocal() and not s.isSiteLocal():
+    return false
+  # TODO: Also check for special reserved ip addresses:
+  # https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml
+  # https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml
+  return true
+
 # TODO: This could be improved to do the clean-up immediatily in case a non
 # whoareyou response does arrive, but we would need to store the AuthTag
 # somewhere
@@ -323,7 +340,11 @@ proc sendFindNode(d: Protocol, toNode: Node, distance: uint32): RequestId =
 
 proc findNode*(d: Protocol, toNode: Node, distance: uint32): Future[seq[Node]] {.async.} =
   let reqId = sendFindNode(d, toNode, distance)
-  result = await d.waitNodes(toNode, reqId)
+  let nodes = await d.waitNodes(toNode, reqId)
+
+  for n in nodes:
+    if validIp(toNode.address.ip, n.address.ip):
+      result.add(n)
 
 proc lookupDistances(target, dest: NodeId): seq[uint32] =
   let td = logDist(target, dest)
@@ -416,6 +437,8 @@ proc revalidateNode*(d: Protocol, n: Node)
       # would be to simply not remove the nodes immediatly but only after x
       # amount of failures.
       discard d.codec.db.deleteKeys(n.id, n.address)
+    else:
+      debug "Revalidation of bootstrap node failed", enr = toURI(n.record)
 
 proc revalidateLoop(d: Protocol) {.async.} =
   try:

tests/p2p/test_discoveryv5.nim

@@ -32,8 +32,8 @@ proc randomPacket(tag: PacketTag): seq[byte] =
   result.add(rlp.encode(authTag))
   result.add(msg)
 
-proc generateNode(privKey = newPrivateKey()): Node =
+proc generateNode(privKey = newPrivateKey(), port: int): Node =
-  let enr = enr.Record.init(1, privKey, none(Address))
+  let enr = enr.Record.init(1, privKey, some(localAddress(port)))
   result = newNode(enr)
 
 suite "Discovery v5 Tests":
@@ -98,7 +98,7 @@ suite "Discovery v5 Tests":
 
     # Generate 1000 random nodes and add to our main node's routing table
     for i in 0..<1000:
-      mainNode.addNode(generateNode())
+      mainNode.addNode(generateNode(port = 20302 + i))
 
     let
       neighbours = mainNode.neighbours(mainNode.localNode.id)
@@ -122,12 +122,12 @@ suite "Discovery v5 Tests":
     # TODO: This could be tested in just a routing table only context
     let
       node = initDiscoveryNode(newPrivateKey(), localAddress(20302), @[])
-      targetNode = generateNode()
+      targetNode = generateNode(port = 20303)
 
     node.addNode(targetNode)
 
     for i in 0..<1000:
-      node.addNode(generateNode())
+      node.addNode(generateNode(port = 20303 + i))
 
     check node.getNode(targetNode.id) == targetNode