diff --git a/eth/p2p/private/p2p_types.nim b/eth/p2p/private/p2p_types.nim index 488fdfe..8dde77c 100644 --- a/eth/p2p/private/p2p_types.nim +++ b/eth/p2p/private/p2p_types.nim @@ -167,19 +167,23 @@ type Disconnecting, Disconnected + # Disconnect message reasons as specified: + # https://github.com/ethereum/devp2p/blob/master/rlpx.md#disconnect-0x01 + # Receiving values that are too large or that are in the enum hole will + # trigger `RlpTypeMismatch` error on deserialization. DisconnectionReason* = enum - DisconnectRequested, - TcpError, - BreachOfProtocol, - UselessPeer, - TooManyPeers, - AlreadyConnected, - IncompatibleProtocolVersion, - NullNodeIdentityReceived, - ClientQuitting, - UnexpectedIdentity, - SelfConnection, - MessageTimeout, + DisconnectRequested = 0x00, + TcpError = 0x01, + BreachOfProtocol = 0x02, + UselessPeer = 0x03, + TooManyPeers = 0x04, + AlreadyConnected = 0x05, + IncompatibleProtocolVersion = 0x06, + NullNodeIdentityReceived = 0x07, + ClientQuitting = 0x08, + UnexpectedIdentity = 0x09, + SelfConnection = 0x0A, + MessageTimeout = 0x0B, SubprotocolReason = 0x10 proc `$`*(peer: Peer): string = $peer.remote diff --git a/eth/p2p/rlpx.nim b/eth/p2p/rlpx.nim index 2b2e05f..c1c0cea 100644 --- a/eth/p2p/rlpx.nim +++ b/eth/p2p/rlpx.nim @@ -708,14 +708,11 @@ proc waitSingleMsg(peer: Peer, MsgType: type): Future[MsgType] {.async.} = "Invalid RLPx message body") elif nextMsgId == 1: # p2p.disconnect - if nextMsgData.isList(): - let reason = DisconnectionReason nextMsgData.listElem(0).toInt(uint32) - await peer.disconnect(reason) - trace "disconnect message received in waitSingleMsg", reason, peer - raisePeerDisconnected("Unexpected disconnect", reason) - else: - raise newException(RlpTypeMismatch, - "List expected, but the source RLP is not a list.") + let reasonList = rlp.read(nextMsgData, DisconnectionReasonList) + let reason = reasonList.value + await peer.disconnect(reason) + trace "disconnect message received in waitSingleMsg", reason, peer + raisePeerDisconnected("Unexpected disconnect", reason) else: warn "Dropped RLPX message", msg = peer.dispatcher.messages[nextMsgId].name diff --git a/eth/rlp.nim b/eth/rlp.nim index 4c9abfc..e43d5b0 100644 --- a/eth/rlp.nim +++ b/eth/rlp.nim @@ -8,6 +8,8 @@ import ./rlp/[writer, object_serialization], ./rlp/priv/defs +from stew/objects import checkedEnumAssign + export writer, object_serialization @@ -293,12 +295,15 @@ proc readImpl(rlp: var Rlp, T: type Integer): Integer = proc readImpl(rlp: var Rlp, T: type[enum]): T = let value = rlp.toInt(int) - if value < ord(T.low) or value > ord(T.high): + + var res: T + if not checkedEnumAssign(res, value): raise newException(RlpTypeMismatch, - "Enum expected, but the source RLP is not in valid range.") - result = type(result)(value) + "Enum value expected, but the source RLP is not in valid range.") rlp.skipElem + res + proc readImpl(rlp: var Rlp, T: type bool): T = result = rlp.toInt(int) != 0 rlp.skipElem diff --git a/tests/p2p/test_rlpx_thunk.json b/tests/p2p/test_rlpx_thunk.json index 5c90ad6..0bad2ae 100644 --- a/tests/p2p/test_rlpx_thunk.json +++ b/tests/p2p/test_rlpx_thunk.json @@ -59,15 +59,25 @@ "error": "RlpTypeMismatch", "description": "listElem to assert on not a single entry list" }, - "Listing single element list with entry off enum range": { - "payload": "01c116", + "DisconnectReason: single element list with entry out off enum range": { + "payload": "01c111", "error": "RlpTypeMismatch", - "description": "Disconnect reason code out of bounds 0..16 (got: 22)" + "description": "Disconnect reason code out of bounds 0..16 (got: 17)" }, - "Listing single element off enum range": { - "payload": "0116", + "DisconnectReason: single element out off enum range": { + "payload": "0111", "error": "RlpTypeMismatch", - "description": "Disconnect reason code out of bounds 0..16 (got: 22)" + "description": "Disconnect reason code out of bounds 0..16 (got: 17)" + }, + "DisconnectReason: single element list with enum hole value": { + "payload": "01c10C", + "error": "RlpTypeMismatch", + "description": "Error on Disconnect reason with enum hole value" + }, + "DisconnectReason: single element with enum hole value": { + "payload": "010C", + "error": "RlpTypeMismatch", + "description": "Error on Disconnect reason with enum hole value" }, "devp2p hello packet version 22 + additional list elements for EIP-8": { "payload": "00f87137916b6e6574682f76302e39312f706c616e39cdc5836574683dc6846d6f726b1682270fb840fda1cff674c90c9a197539fe3dfb53086ace64f83ed7c6eabec741f7f381cc803e52ab2cd55d5569bce4347107a310dfd5f88a010cd2ffd1005ca406f1842877c883666f6f836261720304" diff --git a/tests/rlp/test_api_usage.nim b/tests/rlp/test_api_usage.nim index 57132fe..39cf063 100644 --- a/tests/rlp/test_api_usage.nim +++ b/tests/rlp/test_api_usage.nim @@ -203,16 +203,48 @@ suite "test api usage": test "invalid enum": type MyEnum = enum - foo, - bar + foo = 0x00, + bar = 0x01 var writer = initRlpWriter() - writer.append(2) - writer.append(-1) + writer.append(1) # valid + writer.append(2) # invalid + writer.append(-1) # invalid let bytes = writer.finish() var rlp = rlpFromBytes(bytes) + + check rlp.read(MyEnum) == bar + expect RlpTypeMismatch: discard rlp.read(MyEnum) rlp.skipElem() + expect RlpTypeMismatch: discard rlp.read(MyEnum) + + test "invalid enum - enum with hole": + type + MyEnum = enum + foo = 0x00, + bar = 0x01, + baz = 0x0100 + + var writer = initRlpWriter() + writer.append(1) # valid + writer.append(2) # invalid - enum hole value + writer.append(256) # valid + writer.append(257) # invalid - too large + let bytes = writer.finish() + var rlp = rlpFromBytes(bytes) + + check rlp.read(MyEnum) == bar + + expect RlpTypeMismatch: + discard rlp.read(MyEnum) + rlp.skipElem() + + check rlp.read(MyEnum) == baz + + expect RlpTypeMismatch: + discard rlp.read(MyEnum) + rlp.skipElem()