Wrong lastseen values got written to knownnodes in bmproto #1360

New Issue

2018-10-08T17:04:25+02:00

g1itch commented

2018-10-08 17:04:25 +02:00

(Migrated from github.com)

Hello!

While investigating #1335 I discovered another potential bug.
In the line 380 in bmproto value of seenTime is often wrong. I added a simple check:

    try:
        assert time.time() - seenTime > 0
    except AssertionError:
        logger.warning("node %s: lastseen was: %s!", decodedIP, seenTime)
        seenTime = time.time()

and found values like 281474976710890, 14699749183745158766 or 2448129849.

Hello! While investigating #1335 I discovered another potential bug. In the [line 380 in bmproto](../blob/v0.6/src/network/bmproto.py#L380) value of `seenTime` is often wrong. I added a simple check: ```python try: assert time.time() - seenTime > 0 except AssertionError: logger.warning("node %s: lastseen was: %s!", decodedIP, seenTime) seenTime = time.time() ``` and found values like 281474976710890, 14699749183745158766 or 2448129849.

g1itch commented

2018-10-09 17:10:23 +02:00

(Migrated from github.com)

Today I also found peers with port 0 (:

PeterSurda commented

2018-10-09 17:49:50 +02:00

(Migrated from github.com)

Port 0 used to be filtered, maybe I forgot to move the check when moving to asyncore.

g1itch commented

2018-10-09 17:51:05 +02:00

(Migrated from github.com)

Like this?

diff --git a/src/network/bmproto.py b/src/network/bmproto.py
index 1f1c67bd..ddcbccdd 100644
--- a/src/network/bmproto.py
+++ b/src/network/bmproto.py
@@ -381,14 +381,16 @@ class BMProto(AdvancedDispatcher, ObjectTracker):
             decodedIP = protocol.checkIPAddress(str(ip))
             if stream not in state.streamsInWhichIAmParticipating:
                 continue
-            if decodedIP is not False and seenTime > time.time() - BMProto.addressAlive:
+            if (decodedIP and time.time() - seenTime > 0 and
+                seenTime > time.time() - BMProto.addressAlive and
+                    port > 0):
                 peer = state.Peer(decodedIP, port)
                 try:
                     if knownnodes.knownNodes[stream][peer]["lastseen"] > seenTime:
                         continue
                 except KeyError:
                     pass
-                if len(knownnodes.knownNodes[stream]) < int(BMConfigParser().get("knownnodes", "maxnodes")):
+                if len(knownnodes.knownNodes[stream]) < BMConfigParser().safeGetInt("knownnodes", "maxnodes"):
                     with knownnodes.knownNodesLock:
                         try:
                             knownnodes.knownNodes[stream][peer]["lastseen"] = seenTime

Like this? ```diff diff --git a/src/network/bmproto.py b/src/network/bmproto.py index 1f1c67bd..ddcbccdd 100644 --- a/src/network/bmproto.py +++ b/src/network/bmproto.py @@ -381,14 +381,16 @@ class BMProto(AdvancedDispatcher, ObjectTracker): decodedIP = protocol.checkIPAddress(str(ip)) if stream not in state.streamsInWhichIAmParticipating: continue - if decodedIP is not False and seenTime > time.time() - BMProto.addressAlive: + if (decodedIP and time.time() - seenTime > 0 and + seenTime > time.time() - BMProto.addressAlive and + port > 0): peer = state.Peer(decodedIP, port) try: if knownnodes.knownNodes[stream][peer]["lastseen"] > seenTime: continue except KeyError: pass - if len(knownnodes.knownNodes[stream]) < int(BMConfigParser().get("knownnodes", "maxnodes")): + if len(knownnodes.knownNodes[stream]) < BMConfigParser().safeGetInt("knownnodes", "maxnodes"): with knownnodes.knownNodesLock: try: knownnodes.knownNodes[stream][peer]["lastseen"] = seenTime ```

👍 1

This repo is archived. You cannot comment on issues.