From 944d7a4069012a5980c6ffeec3f2d37d302327e8 Mon Sep 17 00:00:00 2001 From: Jordan Hrycaj Date: Mon, 21 Feb 2022 12:50:20 +0000 Subject: [PATCH 1/6] 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" From 1d8a9cf01c2b45b49f999e0fc8b9a18bf0f34cc9 Mon Sep 17 00:00:00 2001 From: Jordan Hrycaj Date: Fri, 25 Mar 2022 09:11:05 +0000 Subject: [PATCH 2/6] Update eth/p2p/rlpx.nim Co-authored-by: Kim De Mey --- eth/p2p/rlpx.nim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eth/p2p/rlpx.nim b/eth/p2p/rlpx.nim index a3afe96..897cf57 100644 --- a/eth/p2p/rlpx.nim +++ b/eth/p2p/rlpx.nim @@ -43,8 +43,8 @@ type DisconnectionReasonList = object value: DisconnectionReason -proc read(rlp: var Rlp; Q: type DisconnectionReasonList): Q - {.raises: [Defect,RlpError].} = +proc read(rlp: var Rlp; T: type DisconnectionReasonList): T + {.raises: [Defect, RlpError].} = ## Rlp mixin: `DisconnectionReasonList` parser if rlp.isList: # Be strict here: The expression `rlp.read(DisconnectionReasonList)` From 499e5422633f8e10c469288ababd2024d9d9d4fa Mon Sep 17 00:00:00 2001 From: Jordan Hrycaj Date: Fri, 25 Mar 2022 09:11:15 +0000 Subject: [PATCH 3/6] Update eth/p2p/rlpx.nim Co-authored-by: Kim De Mey --- eth/p2p/rlpx.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/p2p/rlpx.nim b/eth/p2p/rlpx.nim index 897cf57..ae7346e 100644 --- a/eth/p2p/rlpx.nim +++ b/eth/p2p/rlpx.nim @@ -50,7 +50,7 @@ proc read(rlp: var Rlp; T: type DisconnectionReasonList): T # 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]) + let a = rlp.read(array[1, DisconnectionReason]) return DisconnectionReasonList(value: a[0]) # Also accepted: a single byte reason code. Is is typically used From 96d07cdea664cfff4961267844af1ae3f340a16b Mon Sep 17 00:00:00 2001 From: Jordan Hrycaj Date: Fri, 25 Mar 2022 09:11:23 +0000 Subject: [PATCH 4/6] Update eth/p2p/rlpx.nim Co-authored-by: Kim De Mey --- eth/p2p/rlpx.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/p2p/rlpx.nim b/eth/p2p/rlpx.nim index ae7346e..39dcb28 100644 --- a/eth/p2p/rlpx.nim +++ b/eth/p2p/rlpx.nim @@ -54,7 +54,7 @@ proc read(rlp: var Rlp; T: type DisconnectionReasonList): T return DisconnectionReasonList(value: a[0]) # Also accepted: a single byte reason code. Is is typically used - # by variants of the reference implentation `Geth` + # by variants of the reference implementation `Geth` if rlp.blobLen <= 1: let n = rlp.read(int) return DisconnectionReasonList(value: n.DisconnectionReason) From 2e41d2892e537fc7682b712ceb72bccdf71d1c77 Mon Sep 17 00:00:00 2001 From: Jordan Hrycaj Date: Fri, 25 Mar 2022 09:11:39 +0000 Subject: [PATCH 5/6] Update eth/p2p/rlpx.nim Co-authored-by: Kim De Mey --- eth/p2p/rlpx.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/p2p/rlpx.nim b/eth/p2p/rlpx.nim index 39dcb28..7fc18f7 100644 --- a/eth/p2p/rlpx.nim +++ b/eth/p2p/rlpx.nim @@ -59,7 +59,7 @@ proc read(rlp: var Rlp; T: type DisconnectionReasonList): T let n = rlp.read(int) return DisconnectionReasonList(value: n.DisconnectionReason) - # Also accepted: a blob of a list (aka object) of reason code. Is is + # Also accepted: a blob of a list (aka object) of reason code. It is # used by `bor`, a `geth` fork var subList = rlp.toBytes.rlpFromBytes if subList.isList: From f214dd8db3cbe01868e4f62793675cc4629cbc2b Mon Sep 17 00:00:00 2001 From: Jordan Hrycaj Date: Fri, 25 Mar 2022 09:11:47 +0000 Subject: [PATCH 6/6] Update eth/p2p/rlpx.nim Co-authored-by: Kim De Mey --- eth/p2p/rlpx.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/p2p/rlpx.nim b/eth/p2p/rlpx.nim index 7fc18f7..fe67874 100644 --- a/eth/p2p/rlpx.nim +++ b/eth/p2p/rlpx.nim @@ -63,7 +63,7 @@ proc read(rlp: var Rlp; T: type DisconnectionReasonList): T # used by `bor`, a `geth` fork var subList = rlp.toBytes.rlpFromBytes if subList.isList: - # Diddo, see above. + # Ditto, see above. let a = subList.read(array[1,DisconnectionReason]) return DisconnectionReasonList(value: a[0]) raise newException(RlpTypeMismatch, "Single entry list expected")