Merge pull request #101 from status-im/fuzz-updates

Fuzzing updates + error fixes

eth/common/eth_types.nim

@@ -289,11 +289,14 @@ proc append*(rlpWriter: var RlpWriter, t: Transaction, a: EthAddress) {.inline.}
     rlpWriter.append(a)
 
 proc read*(rlp: var Rlp, T: typedesc[HashOrStatus]): T {.inline.} =
-  doAssert(rlp.blobLen() == 32 or rlp.blobLen() == 1)
+  if rlp.isBlob() and (rlp.blobLen() == 32 or rlp.blobLen() == 1):
     if rlp.blobLen == 1:
       result = hashOrStatus(rlp.read(uint8) == 1)
     else:
       result = hashOrStatus(rlp.read(Hash256))
+  else:
+    raise newException(RlpTypeMismatch,
+      "HashOrStatus expected, but the source RLP is not a blob of right size.")
 
 proc append*(rlpWriter: var RlpWriter, value: HashOrStatus) {.inline.} =
   if value.isHash:
@@ -326,6 +329,9 @@ proc rlpHash*[T](v: T): Hash256 =
 func blockHash*(h: BlockHeader): KeccakHash {.inline.} = rlpHash(h)
 
 proc notImplemented =
+  when defined(afl) or defined(libFuzzer):
+    discard
+  else:
     doAssert false, "Method not implemented"
 
 template hasData*(b: Blob): bool = b.len > 0

eth/p2p/rlpx.nim

@@ -23,7 +23,13 @@ type
 
   ResponderWithoutId*[MsgType] = distinct Peer
 
+  # We need these two types in rlpx/devp2p as no parameters or single parameters
+  # are not getting encoded in an rlp list.
+  # TODO: we could generalize this in the protocol dsl but it would need an
+  # `alwaysList` flag as not every protocol expects lists in these cases.
   EmptyList = object
+  DisconnectionReasonList = object
+    value: DisconnectionReason
 
 const
   devp2pVersion* = 4
@@ -240,7 +246,10 @@ proc invokeThunk*(peer: Peer, msgId: int, msgData: var Rlp): Future[void] =
       "RLPx message with an invalid id " & $msgId &
       " on a connection supporting " & peer.dispatcher.describeProtocols)
 
-  if msgId >= peer.dispatcher.messages.len: invalidIdError()
+  # msgId can be negative as it has int as type and gets decoded from rlp
+  if msgId >= peer.dispatcher.messages.len or msgId < 0: invalidIdError()
+  if peer.dispatcher.messages[msgId].isNil: invalidIdError()
+
   let thunk = peer.dispatcher.messages[msgId].thunk
   if thunk == nil: invalidIdError()
 
@@ -457,10 +466,14 @@ 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.")
     else:
       warn "Dropped RLPX message",
            msg = peer.dispatcher.messages[nextMsgId].name
@@ -512,12 +525,6 @@ proc dispatchMessages*(peer: Peer) {.async.} =
     except PeerDisconnected:
       return
 
-    if msgId == 1: # p2p.disconnect
-      let reason = msgData.listElem(0).toInt(uint32).DisconnectionReason
-      trace "disconnect message received in dispatchMessages", reason, peer
-      await peer.disconnect(reason, false)
-      break
-
     try:
       await peer.invokeThunk(msgId, msgData)
     except RlpError:
@@ -781,11 +788,12 @@ p2pProtocol devp2p(version = 0, rlpxName = "p2p"):
              listenPort: uint,
              nodeId: array[RawPublicKeySize, byte])
 
-  proc sendDisconnectMsg(peer: Peer, reason: DisconnectionReason)
+  proc sendDisconnectMsg(peer: Peer, reason: DisconnectionReasonList) =
+    trace "disconnect message received", reason=reason.value, peer
+    await peer.disconnect(reason.value, false)
 
   # Adding an empty RLP list as the spec defines.
   # The parity client specifically checks if there is rlp data.
-  # TODO: can we do this in the macro instead?
   proc ping(peer: Peer, emptyList: EmptyList) =
     discard peer.pong(EmptyList())
 
@@ -830,7 +838,7 @@ proc disconnect*(peer: Peer, reason: DisconnectionReason, notifyOtherPeer = fals
       traceAwaitErrors callDisconnectHandlers(peer, reason)
 
     if notifyOtherPeer and not peer.transport.closed:
-      var fut = peer.sendDisconnectMsg(reason)
+      var fut = peer.sendDisconnectMsg(DisconnectionReasonList(value: reason))
       yield fut
       if fut.failed:
         debug "Failed to deliver disconnect message", peer

eth/rlp.nim

@@ -272,7 +272,15 @@ iterator items*(self: var Rlp): var Rlp =
     position = elemEnd
 
 proc listElem*(self: Rlp, i: int): Rlp =
-  let payload = bytes.slice(position + payloadOffset())
+  doAssert isList()
+  let
+    payloadOffset = payloadOffset()
+
+  # This will only check if there is some data, not if it is correct according
+  # to list length. Could also run here payloadBytesCount() instead.
+  if position + payloadOffset + 1 > bytes.len: eosError()
+
+  let payload = bytes.slice(position + payloadOffset)
   result = rlpFromBytes payload
   var pos = 0
   while pos < i and result.hasData:
@@ -364,6 +372,9 @@ proc readImpl(rlp: var Rlp, T: type[object|tuple],
   mixin enumerateRlpFields, read
 
   if wrappedInList:
+    if not rlp.isList:
+      raise newException(RlpTypeMismatch,
+                        "List expected, but the source RLP is not a list.")
     var
       payloadOffset = rlp.payloadOffset()
       payloadEnd = rlp.position + payloadOffset + rlp.payloadBytesCount()

tests/fuzzing/config.nims

@@ -7,6 +7,7 @@ proc aflSwitches() =
   switch("out", "fuzz-afl")
 
 proc libFuzzerSwitches() =
+  switch("define", "libFuzzer")
   switch("noMain", "")
   switch("cc", "clang")
   switch("passC", "-fsanitize=fuzzer,address")

tests/fuzzing/fuzz_helpers.nim

@@ -23,7 +23,8 @@ const
              "--clang.linkerexe=afl-clang"
   aflClangFast = "--cc=clang " &
                  "--clang.exe=afl-clang-fast " &
-                 "--clang.linkerexe=afl-clang-fast"
+                 "--clang.linkerexe=afl-clang-fast " &
+                 "-d:clangfast"
   libFuzzerClang = "--cc=clang " &
                    "--passC='-fsanitize=fuzzer,address' " &
                    "--passL='-fsanitize=fuzzer,address'"
@@ -43,7 +44,7 @@ type
     clangFast = aflClangFast
 
 proc aflCompile*(target: string, c: Compiler) =
-  let aflOptions = &"-d:standalone -d:noSignalHandler {$c}"
+  let aflOptions = &"-d:afl -d:noSignalHandler {$c}"
   let compileCmd = &"nim c {defaultFlags} {aflOptions} {target.quoteShell()}"
   exec compileCmd
 
@@ -65,7 +66,7 @@ proc aflExec*(target: string, inputDir: string, resultsDir: string,
   exec fuzzCmd
 
 proc libFuzzerCompile*(target: string) =
-  let libFuzzerOptions = &"--noMain {libFuzzerClang}"
+  let libFuzzerOptions = &"-d:libFuzzer --noMain {libFuzzerClang}"
   let compileCmd = &"nim c {defaultFlags} {libFuzzerOptions} {target.quoteShell()}"
   exec compileCmd
 

tests/fuzzing/fuzztest.nim

@@ -3,7 +3,7 @@ import streams, posix, strutils, chronicles, macros, stew/ranges/ptr_arith
 template fuzz(body) =
   # For code we want to fuzz, SIGSEGV is needed on unwanted exceptions.
   # However, this is only needed when fuzzing with afl.
-  when defined(standalone):
+  when defined(afl):
     try:
       body
     except Exception as e:
@@ -12,7 +12,7 @@ template fuzz(body) =
   else:
     body
 
-proc readStdin*(): seq[byte] =
+proc readStdin(): seq[byte] =
   # Read input from stdin (fastest for AFL)
   let s = newFileStream(stdin)
   if s.isNil:
@@ -29,7 +29,7 @@ proc NimMain() {.importc: "NimMain".}
 
 # The default init, gets redefined when init template is used.
 template initImpl(): untyped =
-  when defined(standalone):
+  when not defined(libFuzzer):
     discard
   else:
     proc fuzzerInit(): cint {.exportc: "LLVMFuzzerInitialize".} =
@@ -38,7 +38,15 @@ template initImpl(): untyped =
       return 0
 
 template init*(body: untyped) =
-  when defined(standalone):
+  ## Init block to do any initialisation for the fuzzing test.
+  ##
+  ## For AFL this is currently only cosmetic and will be run each time, before
+  ## the test block.
+  ##
+  ## For libFuzzer this will only be run once. So only put data which is
+  ## stateless or make sure everything gets properply reset for each new run in
+  ## the test block.
+  when not defined(libFuzzer):
     template initImpl(): untyped = fuzz: `body`
   else:
     template initImpl() =
@@ -50,9 +58,13 @@ template init*(body: untyped) =
         return 0
 
 template test*(body: untyped): untyped =
+  ## Test block to do the actual test that will be fuzzed in a loop.
+  ##
+  ## Within this test block there is access to the payload OpenArray which
+  ## contains the payload provided by the fuzzer.
   mixin initImpl
   initImpl()
-  when defined(standalone):
+  when not defined(libFuzzer):
     var payload {.inject.} = readStdin()
 
     fuzz: `body`
@@ -63,3 +75,21 @@ template test*(body: untyped): untyped =
         makeOpenArray(data, len)
 
       `body`
+
+when defined(clangfast):
+  ## Can be used for deferred instrumentation.
+  ## Should be placed on a suitable location in the code where the delayed
+  ## cloning can take place (e.g. NOT after creation of threads)
+  proc aflInit*() {.importc: "__AFL_INIT", noDecl.}
+  ## Can be used for persistent mode.
+  ## Should be used as value for controlling a loop around a test case.
+  ## Test case should be able to handle repeated inputs. No repeated fork() will
+  ## be done.
+  # TODO: Lets use this in the test block when afl-clang-fast is used?
+  proc aflLoopImpl(count: cuint): cint {.importc: "__AFL_LOOP", noDecl.}
+  template aflLoop*(body: untyped): untyped =
+    while aflLoopImpl(1000) != 0:
+      `body`
+else:
+  proc aflInit*() = discard
+  template aflLoop*(body: untyped): untyped = `body`

tests/fuzzing/readme.md

@@ -0,0 +1,204 @@
+# Fuzzing
+## tldr:
+* [Install afl](#Install-afl).
+* Create a testcase.
+* Run: `nim fuzz.nims afl testfolder/testcase.nim`
+
+Or
+
+* [Install libFuzzer](#Install-libFuzzer) (comes with LLVM).
+* Create a testcase.
+* Run: `nim fuzz.nims libFuzzer testfolder/testcase.nim`
+
+## Fuzzing Helpers
+There are two convenience templates which will help you set up a quick fuzzing
+test.
+
+These are the mandatory `test` block and the optional `init` block.
+
+Example usage:
+```nim
+test:
+  var rlp = rlpFromBytes(@payload.toRange)
+  discard rlp.inspect()
+```
+
+Any unhandled `Exception` will result in a failure of the testcase. If certain
+`Exception`s are to be allowed to occur within the test, they should be caught.
+
+E.g.:
+```nim
+test:
+  try:
+    var rlp = rlpFromBytes(@payload.toRange)
+    discard rlp.inspect()
+  except RlpError:
+    debug "Inspect failed", err = getCurrentExceptionMsg()
+```
+
+## Supported Fuzzers
+The two templates can prepare the code for both
+[afl](http://lcamtuf.coredump.cx/afl/) and
+[libFuzzer](http://llvm.org/docs/LibFuzzer.html).
+
+You will need to install first the fuzzer you want to use.
+### Install afl
+```sh
+# Ubuntu / Debian
+sudo apt-get install afl
+
+# Fedora
+dnf install american-fuzzy-lop
+# for usage with clang & clang-fast you will have to install
+# american-fuzzy-lop-clang or american-fuzzy-lop-clang-fast
+
+# Arch Linux
+pacman -S afl
+
+# NixOS
+nix-env -i afl
+
+```
+
+### Install libFuzzer
+
+LibFuzzer is part of llvm and will be installed together with llvm-libs in
+recent versions. Installing clang should install llvm-libs.
+```sh
+# Ubuntu / Debian
+sudo apt-get install clang
+
+# Fedora
+dnf install clang
+
+# Arch Linux
+pacman -S clang
+
+# NixOS
+nix-env -iA nixos.clang_7 nixos.llvm_7
+```
+
+## Compiling & Starting the Fuzzer
+### Scripted helper
+There is a nimscript helper to compile & start the fuzzer:
+```sh
+# for afl
+nim fuzz.nims afl testcase.nim
+
+# for libFuzzer
+nim fuzz.nims libFuzzer testcase.nim
+```
+### Manually with afl
+#### Compiling
+With gcc:
+```sh
+nim c -d:afl -d:release -d:chronicles_log_level=fatal -d:noSignalHandler --cc=gcc --gcc.exe=afl-gcc --gcc.linkerexe=afl-gcc testcase.nim
+```
+The `afl` define is specifically required for the `init` and `test`
+templates.
+
+You typically want to fuzz in `-d:release` and probably also want to lower down
+the logging. But this is not strictly necessary.
+
+There is also a nimscript task in `config.nims` for this:
+```
+nim c build_afl testcase.nim
+```
+
+With clang:
+```sh
+# afl-clang
+nim c -d:afl -d:noSignalHandler --cc=clang --clang.exe=afl-clang --clang.linkerexe=afl-clang ftestcase.nim
+# afl-clang-fast
+nim c -d:afl -d:noSignalHandler --cc=clang --clang.exe=afl-clang-fast --clang.linkerexe=afl-clang-fast testcase.nim
+```
+
+#### Starting the Fuzzer
+
+To start the fuzzer:
+```sh
+afl-fuzz -i input -o results -- ./testcase
+```
+
+To rerun it without losing previous results/corpus:
+```sh
+afl-fuzz -i - -o results -- ./testcase
+```
+
+To run several parallel fuzzing sessions:
+```sh
+# Start master fuzzer
+afl-fuzz -i input -o results -M fuzzer01 -- ./testcase
+# Start slaves (usually 1 per core available)
+afl-fuzz -i input -o results -S fuzzer02 -- ./testcase
+afl-fuzz -i input -o results -S fuzzer03 -- ./testcase
+# add more if needed
+```
+
+When compiled with `-d:afl` the resulting application can also be run
+manually by providing it input data, e.g.:
+```sh
+./testcase < testfile
+```
+
+During debugging you might not want the testcase to generate a segmentation
+fault on exceptions. You can do this by rebuilding the test without the `-d:afl`
+flag. Changing to `-d:debug` will also help but might also change the
+behaviour.
+
+### Manually with libFuzzer
+#### Compiling
+```sh
+nim c -d:libFuzzer -d:release -d:chronicles_log_level=fatal --noMain --cc=clang --passC="-fsanitize=fuzzer" --passL="-fsanitize=fuzzer" testcase.nim
+```
+
+The `libFuzzer` define is specifically required for the `init` and `test`
+templates.
+
+You typically want to fuzz in `-d:release` and probably also want to lower down
+the logging. But this is not strictly necessary.
+
+There is also a nimscript task in `config.nims` for compiling:
+```
+nim c build_libFuzzer testcase.nim
+```
+
+#### Starting the Fuzzer
+Starting the fuzzer is as simple as running the compiled program:
+```sh
+./testcase corpus_dir -runs=1000000
+```
+
+To see the available options:
+```sh
+./testcase test=1
+```
+
+Parallel fuzzing on 8 cores:
+```sh
+./fuzz-libfuzzer -jobs=8 -workers=8
+```
+
+You can also use the application to verify a specific test case:
+```sh
+./testcase input_file
+```
+
+## Additional notes
+The `init` template, when used with **afl**, is only cosmetic. It will be
+run before each test block, compared to libFuzzer, where it will be run only
+once.
+
+In case of using afl with `alf-clang-fast` you can make use of `aflInit()` proc
+and `aflLoop()` template.
+
+`aflInit()` will allow using what is called deferred instrumentation. Basically,
+the forking of the process will only happen after this call, where normally it
+is done right before `main()`.
+
+`aflLoop:` will allow for (experimental) persistant mode. It will run the test
+in loop (1000 iterations) with different payloads. This is more comparable with
+libFuzzer.
+
+These calls are enabled with `-d:clangfast`, and have to be manually added.
+They are currently not part of the `test` or `init` templates.

tests/fuzzing/rlpx/thunk.nim

@@ -0,0 +1,35 @@
+import
+  chronos, eth/p2p, eth/p2p/rlpx, eth/p2p/private/p2p_types,
+  eth/p2p/rlpx_protocols/[whisper_protocol, eth_protocol],
+  ../fuzztest, ../p2p/p2p_test_helper
+
+proc recvMsgMock(msg: openArray[byte]): tuple[msgId: int, msgData: Rlp] =
+  var rlp = rlpFromBytes(@msg.toRange)
+
+  let msgid = rlp.read(int)
+  return (msgId, rlp)
+
+var
+  node1: EthereumNode
+  node2: EthereumNode
+  peer: Peer
+
+# This is not a good example of a fuzzing test and it would be much better
+# to mock more to get rid of anything sockets, async, etc.
+# However, it can and has provided reasonably quick results anyhow.
+init:
+  node1 = setupTestNode(eth, Whisper)
+  node2 = setupTestNode(eth, Whisper)
+
+  node2.startListening()
+  peer = waitFor node1.rlpxConnect(newNode(initENode(node2.keys.pubKey,
+                                                     node2.address)))
+
+test:
+  aflLoop: # This appears to have unstable results with afl-clang-fast, probably
+           # because of undeterministic behaviour due to usage of network/async.
+    try:
+      var (msgId, msgData) = recvMsgMock(payload)
+      waitFor peer.invokeThunk(msgId.int, msgData)
+    except CatchableError as e:
+      debug "Test caused CatchableError", exception=e.name, trace=e.repr, msg=e.msg