From 944d7a4069012a5980c6ffeec3f2d37d302327e8 Mon Sep 17 00:00:00 2001
From: Jordan Hrycaj <jordan@status.im>
Date: Mon, 21 Feb 2022 12:50:20 +0000
Subject: [PATCH] Mitigating RLP annoyances

why:
  Rlp errors throw exceptions which cause the dispatcher loop to
  terminate the current session immediately.

details:
  The DisconnectionReasonList message requires a single entry list.
  Observed and now accepted deviations are:

  Geth: single byte number

  bor(a Geth fork): blobbed single entry list containing a number
---
 eth/p2p/rlpx.nim               | 28 +++++++++++++++++++++++++++-
 tests/p2p/test_rlpx_thunk.json | 11 ++++++++---
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/eth/p2p/rlpx.nim b/eth/p2p/rlpx.nim
index bc0bb33..a3afe96 100644
--- a/eth/p2p/rlpx.nim
+++ b/eth/p2p/rlpx.nim
@@ -43,6 +43,32 @@ type
   DisconnectionReasonList = object
     value: DisconnectionReason
 
+proc read(rlp: var Rlp; Q: type DisconnectionReasonList): Q
+    {.raises: [Defect,RlpError].} =
+  ## Rlp mixin: `DisconnectionReasonList` parser
+  if rlp.isList:
+    # Be strict here: The expression `rlp.read(DisconnectionReasonList)`
+    # accepts lists with at least one item. The array expression wants
+    # exactly one item.
+    let a = rlp.read(array[1,DisconnectionReason])
+    return DisconnectionReasonList(value: a[0])
+
+  # Also accepted: a single byte reason code. Is is typically used
+  # by variants of the reference implentation `Geth`
+  if rlp.blobLen <= 1:
+    let n = rlp.read(int)
+    return DisconnectionReasonList(value: n.DisconnectionReason)
+
+  # Also accepted: a blob of a list (aka object) of reason code. Is is
+  # used by `bor`, a `geth` fork
+  var subList = rlp.toBytes.rlpFromBytes
+  if subList.isList:
+    # Diddo, see above.
+    let a = subList.read(array[1,DisconnectionReason])
+    return DisconnectionReasonList(value: a[0])
+  raise newException(RlpTypeMismatch, "Single entry list expected")
+
+
 const
   devp2pVersion* = 4
   maxMsgSize = 1024 * 1024 * 10
@@ -518,7 +544,7 @@ proc checkedRlpRead(peer: Peer, r: var Rlp, MsgType: type):
             peer = peer,
             dataType = MsgType.name,
             exception = e.msg
-            # rlpData = r.inspect
+            # rlpData = r.inspect -- don't use (might crash)
 
       raise e
 
diff --git a/tests/p2p/test_rlpx_thunk.json b/tests/p2p/test_rlpx_thunk.json
index f0eb323..a57e77f 100644
--- a/tests/p2p/test_rlpx_thunk.json
+++ b/tests/p2p/test_rlpx_thunk.json
@@ -49,10 +49,15 @@
     "error": "MalformedRlpError",
     "description": "listElem to error on invalid size encoding"
   },
-  "Listing elements when not a list": {
-    "payload": "010a",
+  "Listing single element list when having more entries": {
+    "payload": "01c20420",
     "error": "RlpTypeMismatch",
-    "description": "listElem to assert on not a list"
+    "description": "listElem to assert on not a single entry list"
+  },
+  "Listing single element list when having empty list": {
+    "payload": "01c0",
+    "error": "RlpTypeMismatch",
+    "description": "listElem to assert on not a single entry list"
   },
   "devp2p hello packet version 22 + additional list elements for EIP-8": {
     "payload": "00f87137916b6e6574682f76302e39312f706c616e39cdc5836574683dc6846d6f726b1682270fb840fda1cff674c90c9a197539fe3dfb53086ace64f83ed7c6eabec741f7f381cc803e52ab2cd55d5569bce4347107a310dfd5f88a010cd2ffd1005ca406f1842877c883666f6f836261720304"