From 8761ea3222f8d4fbd7ebae6755665e791499d7f2 Mon Sep 17 00:00:00 2001 From: Jordan Hrycaj Date: Thu, 16 Jun 2022 16:23:07 +0100 Subject: [PATCH] Fix stability issues (#512) * Fix stability issues why: Handling malformed messages typically raises `RangeError` exceptions when de-serialising RLP, or decoding message data. This is an (incomplete) attempt to weed out some out it driven by real live tests. remark: Employing the new `snap` protocol there might be different views on what the messages really contain (currently specs are more a hint.) * Update RLP exception handling * Undo effect-less patch why: problem occurred somewhere above the try/catch handler * Using `checkedEnumAssign()` for RLP enum --- eth/common/eth_types.nim | 16 +++++++++++++--- eth/p2p/rlpx.nim | 19 +++++++++++-------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/eth/common/eth_types.nim b/eth/common/eth_types.nim index 7e771a4..7c22e06 100644 --- a/eth/common/eth_types.nim +++ b/eth/common/eth_types.nim @@ -3,6 +3,9 @@ import stew/[endians2, byteutils], chronicles, stint, nimcrypto/[keccak, hash], ../rlp, ../trie/[trie_defs, db] +from stew/objects + import checkedEnumAssign + export stint, read, append, KeccakHash, rlp, options @@ -454,14 +457,21 @@ proc readTxTyped(rlp: var Rlp, tx: var Transaction) {.inline.} = let txType = rlp.getByteValue rlp.position += 1 - case TxType(txType): + var txVal: TxType + if checkedEnumAssign(txVal, txType): + case txVal: of TxEip2930: rlp.readTxEip2930(tx) + return of TxEip1559: rlp.readTxEip1559(tx) + return else: - raise newException(UnsupportedRlpError, - "TypedTransaction type must be 1 or 2 in this version, got " & $txType) + discard + + raise newException(UnsupportedRlpError, + "TypedTransaction type must be 1 or 2 in this version, got " & $txType) + proc read*(rlp: var Rlp, T: type Transaction): T = # Individual transactions are encoded and stored as either `RLP([fields..])` diff --git a/eth/p2p/rlpx.nim b/eth/p2p/rlpx.nim index 6af208b..85906f3 100644 --- a/eth/p2p/rlpx.nim +++ b/eth/p2p/rlpx.nim @@ -62,22 +62,25 @@ 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. - return DisconnectionReasonList( - value: rlp.read(array[1,DisconnectionReason])[0]) + if rlp.rawData.len < 3: + # avoids looping through all items when parsing for an overlarge array + return DisconnectionReasonList( + value: rlp.read(array[1,DisconnectionReason])[0]) # Also accepted: a single byte reason code. Is is typically used # by variants of the reference implementation `Geth` - if rlp.blobLen <= 1: + elif rlp.blobLen <= 1: return DisconnectionReasonList( value: rlp.read(DisconnectionReason)) # 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: - # Ditto, see above. - return DisconnectionReasonList( - value: subList.read(array[1,DisconnectionReason])[0]) + elif rlp.blobLen < 4: + var subList = rlp.toBytes.rlpFromBytes + if subList.isList: + # Ditto, see above. + return DisconnectionReasonList( + value: subList.read(array[1,DisconnectionReason])[0]) raise newException(RlpTypeMismatch, "Single entry list expected")