Fix enr deserialization crash + more tests + fuzz test (#274)

* Fix enr deserialization crash + more tests + fuzz test

* CI: Install depsOnly

.appveyor.yml

@@ -72,7 +72,7 @@ install:
 
 build_script:
   - cd C:\projects\%APPVEYOR_PROJECT_SLUG%
-  - bash -c "nimble install -y"
+  - bash -c "nimble install -y --depsOnly"
 
 test_script:
   - nimble test

.travis.yml

@@ -44,5 +44,5 @@ install:
   - cd ../../..
 
 script:
-  - nimble install -y
+  - nimble install -y --depsOnly
   - nimble test

eth/p2p/discoveryv5/enr.nim

@@ -457,7 +457,7 @@ proc `==`*(a, b: Record): bool = a.raw == b.raw
 
 proc read*(rlp: var Rlp, T: typedesc[Record]):
     T {.inline, raises:[RlpError, ValueError, Defect].} =
-  if not result.fromBytes(rlp.rawData):
+  if not rlp.hasData() or not result.fromBytes(rlp.rawData):
     # TODO: This could also just be an invalid signature, would be cleaner to
     # split of RLP deserialisation errors from this.
     raise newException(ValueError, "Could not deserialize")

tests/fuzzing/enr/fuzz_enr.nim

@@ -0,0 +1,29 @@
+import
+  testutils/fuzzing, stew/byteutils,
+  eth/rlp, eth/p2p/discoveryv5/enr
+
+test:
+  block:
+    # This is fuzzing the full ENR deserialisation. As ENRs contain a signature
+    # this will practically always fail. So the second (encoding) steps will
+    # never be reached.
+    # However, as the signature checking is done at the end, a big part of the
+    # parsing will still be fuzzed.
+    let decoded = try: rlp.decode(payload, enr.Record)
+                  except RlpError as e:
+                        debug "decode failed", err = e.msg
+                        break
+                  except ValueError as e:
+                        debug "decode failed", err = e.msg
+                        break
+
+    let encoded = try: rlp.encode(decoded)
+                  except RlpError as e:
+                    debug "decode failed", err = e.msg
+                    doAssert(false, "decoding worked but encoding failed")
+                    break
+    if encoded != payload.toOpenArray(0, encoded.len - 1):
+        echo "payload: ", toHex(payload.toOpenArray(0, encoded.len - 1))
+        echo "encoded: ", toHex(encoded)
+
+        doAssert(false, "re-encoded result does not equal original payload")

tests/fuzzing/enr/generate.nim

@@ -0,0 +1,30 @@
+import
+  streams, os, strutils, options,
+  stew/shims/net,
+  eth/keys, eth/p2p/discoveryv5/enr
+
+template sourceDir: string = currentSourcePath.rsplit(DirSep, 1)[0]
+const inputsDir = sourceDir / "corpus"
+
+proc toFile(data: seq[byte], fn: string) =
+  var s = newFileStream(fn, fmWrite)
+  for x in data:
+    s.write(x)
+  s.close()
+
+proc generate() =
+  let
+    rng = newRng()
+    privKey = PrivateKey.random(rng[])
+    ip = some(ValidIpAddress.init("127.0.0.1"))
+    port = Port(20301)
+
+  block:
+    let record = enr.Record.init(1, privKey, ip, port, port)[]
+    record.raw.toFile(inputsDir / "enr1")
+  block:
+    let record = enr.Record.init(1, privKey, ip, port, port, [toFieldPair("test", 1'u)])[]
+    record.raw.toFile(inputsDir / "enr2")
+
+discard existsOrCreateDir(inputsDir)
+generate()

tests/p2p/test_enr.nim

@@ -1,7 +1,7 @@
 import
   unittest, options, sequtils,
   nimcrypto/utils, stew/shims/net,
-  eth/p2p/enode, eth/p2p/discoveryv5/enr, eth/keys
+  eth/p2p/enode, eth/p2p/discoveryv5/enr, eth/[keys, rlp]
 
 let rng = newRng()
 
@@ -10,28 +10,51 @@ suite "ENR":
     var pk = PrivateKey.fromHex(
       "5d2908f3f09ea1ff2e327c3f623159639b00af406e9009de5fd4b910fc34049d")[]
     var r = initRecord(123, pk, {"udp": 1234'u, "ip": [byte 5, 6, 7, 8]})[]
-    doAssert($r == """(id: "v4", ip: 0x05060708, secp256k1: 0x02E51EFA66628CE09F689BC2B82F165A75A9DDECBB6A804BE15AC3FDF41F3B34E7, udp: 1234)""")
+    check($r == """(id: "v4", ip: 0x05060708, secp256k1: 0x02E51EFA66628CE09F689BC2B82F165A75A9DDECBB6A804BE15AC3FDF41F3B34E7, udp: 1234)""")
     let uri = r.toURI()
     var r2: Record
     let sigValid = r2.fromURI(uri)
-    doAssert(sigValid)
+    check(sigValid)
-    doAssert($r2 == $r)
+    check($r2 == $r)
+    check(r2.raw == r.raw)
+
+  test "RLP serialisation":
+    var pk = PrivateKey.fromHex(
+      "5d2908f3f09ea1ff2e327c3f623159639b00af406e9009de5fd4b910fc34049d")[]
+    var r = initRecord(123, pk, {"udp": 1234'u, "ip": [byte 5, 6, 7, 8]})[]
+    check($r == """(id: "v4", ip: 0x05060708, secp256k1: 0x02E51EFA66628CE09F689BC2B82F165A75A9DDECBB6A804BE15AC3FDF41F3B34E7, udp: 1234)""")
+    let encoded = rlp.encode(r)
+    let decoded = rlp.decode(encoded, enr.Record)
+    check($decoded == $r)
+    check(decoded.raw == r.raw)
+
+  test "RLP deserialisation without data":
+    expect ValueError:
+      let decoded = rlp.decode([], enr.Record)
+
+    var r: Record
+    check not fromBytes(r, [])
+
+  test "Base64 dserialsation without data":
+    var r: Record
+    let sigValid = r.fromURI("enr:")
+    check(not sigValid)
 
   test "Parsing":
     var r: Record
     let sigValid = r.fromBase64("-IS4QHCYrYZbAKWCBRlAy5zzaDZXJBGkcnh4MHcBFZntXNFrdvJjX04jRzjzCBOonrkTfj499SZuOh8R33Ls8RRcy5wBgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQPKY0yuDUmstAHYpMa2_oxVtw0RW_QAdpzBQA8yWM0xOIN1ZHCCdl8")
-    doAssert(sigValid)
+    check(sigValid)
-    doAssert($r == """(id: "v4", ip: 0x7F000001, secp256k1: 0x03CA634CAE0D49ACB401D8A4C6B6FE8C55B70D115BF400769CC1400F3258CD3138, udp: 30303)""")
+    check($r == """(id: "v4", ip: 0x7F000001, secp256k1: 0x03CA634CAE0D49ACB401D8A4C6B6FE8C55B70D115BF400769CC1400F3258CD3138, udp: 30303)""")
 
   test "Bad base64":
     var r: Record
     let sigValid = r.fromURI("enr:-IS4QHCYrYZbAKWCBRlAy5zzaDZXJBGkcnhMHcBFZntXNFrdv*jX04jRzjzCBOonrkTfj499SZuOh8R33Ls8RRcy5wBgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQPKY0yuDUmstAHYpMa2_oxVtw0RW_QAdpzBQA8yWM0xOIN1ZHCCdl8")
-    doAssert(not sigValid)
+    check(not sigValid)
 
   test "Bad rlp":
     var r: Record
     let sigValid = r.fromBase64("-IS4QHCYrYZbAKWCBRlAy5zzaDZXJBGkcnh4MHcBFZntXNFrdvJjX04jRzjzCBOOnrkTfj499SZuOh8R33Ls8RRcy5wBgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQPKY0yuDUmstAHYpMa2_oxVtw0RW_QAdpzBQA8yWM0xOIN1ZHCCdl8")
-    doAssert(not sigValid)
+    check(not sigValid)
 
   test "Create from ENode address":
     let