nimbus-eth1 icon indicating copy to clipboard operation
nimbus-eth1 copied to clipboard

[Security] Fuzzing and various hardened/sanitized code checks

Open mratsim opened this issue 5 years ago • 15 comments

Given the potential fallout of security issues in Nimbus, I'd like to keep track of solutions to make sure we're comfortable with shipping this:

  • American Fuzzy Loop: https://github.com/nim-lang/Nim/wiki/Fuzzing-your-nim-code-to-rabbit-out-all-the-hard-bugs
  • TIS interpreter: https://github.com/TrustInSoft/tis-interpreter

mratsim avatar Oct 02 '18 15:10 mratsim

LLVM has a lot of sanitizers that could be interesting: https://clang.llvm.org/docs/index.html

arnetheduck avatar Oct 02 '18 15:10 arnetheduck

Looking into this one.

corpetty avatar Jul 25 '19 17:07 corpetty

I agree with @arnetheduck, LLVM's sanitizers are great as well! You may also want to look at libFuzzer, which is also part of the LLVM ecosystem.

  • AFL can be hard to instrument code, but if you can get that working it would be great. I checked out the Nim docs on AFL, and it looks rather straight forward.
  • TIS & Frama-C require a bit of work, but if you can get them running you'll probably have deep insights.
  • Not fuzzing related, but if you're compiling Nim to C, you can use NASA's Ikos, which, like TIS, is an abstract interpreter.
  • I've had great results with very "dumb" (or unguided) fuzzers, like radamsa. I can coördinate with @corpetty on my Radamsa fuzzing rig for both network & local code if you'd like.
  • I'd also recommend Property checkers like QuickCheck for Nim via QuickTest.

Basically, what I recommend would be this:

  • simple static checks (like rats, splint and so on) to catch painfully obvious bugs
  • an unguided fuzzer to find obvious bugs quickly
  • some sort of guided fuzzer to find more subtle bugs that require more involved work
  • verification via symbolic execution or abstract interpretation for very deep bugs or edge cases

lojikil avatar Jul 25 '19 17:07 lojikil

oh, and another point: make sure your fuzzer generates test cases that you can reuse for future re-verification or regression testing. I very often have radamsa generate files on the file system, and I save the seed and any crashing test cases. In this way, the client can easily see which test case crashed, and re-run the proof of concept.

lojikil avatar Jul 25 '19 17:07 lojikil

Good info, thanks!

Some basic work for unguided fuzzing (or lets say slightly guided) with afl has been done on the discovery code and also a bit on the json-rpc code.

https://github.com/status-im/nim-eth/tree/master/tests/fuzzing

Found quite some bugs already with this simple setup. I was still planning on improving and continuing that on other parts of the code.

kdeme avatar Jul 26 '19 14:07 kdeme

➤ Giovanni Petrantoni commented:

LLVM sanitizers are indeed a 1 minute 1 line switch on (ASAN, UBSAN), they work on mac and linux only tho, those should be part of the testing I guess. GAs and fuzzers are nice too but need some setup. Also the good old Valgrind is invaluable.

0xc1c4da avatar Feb 04 '20 17:02 0xc1c4da

https://github.com/status-im/nim-beacon-chain/tree/master/nfuzz is used for differential fuzzing of the beacon chain spec - there's a fuzzer that compares the results of different client implementations - similar is being done via evmc: https://github.com/ethereum/evmone/tree/master/test/fuzzer

arnetheduck avatar Feb 04 '20 18:02 arnetheduck

nim-eth also has some fuzzing targets and some code to make setup easier: https://github.com/status-im/nim-eth/tree/master/tests/fuzzing
But more work is needed there.

kdeme avatar Feb 04 '20 21:02 kdeme

Btw something extremely important that I realized only recently is that any GC or allocator should be absolutely disabled when doing this kind of checks ( can have a run with them on to check them too, but not so crucial ). As they completely hide potential huge memory issues, such as even simple issues of alignments. I recently had a buggy codebase for months just because my smart allocator was pre-allocating enough memory to forget about the issue and let ASAN/Valgrind/UBSAN pass fine :)

sinkingsugar avatar Feb 05 '20 00:02 sinkingsugar

I've already mentionned it to @Araq but even beyond Nim GC, Nim internals should be gradually sanitized as they interfere/produce false positives:

  • Address Sanitizer and empty strings produce a global buffer overflow warning (https://github.com/mratsim/weave/issues/80#issue-543593052)
  • ThreadSanitizer is full of false positives @Milerius (https://github.com/Milerius/atomic_dex_playground/tree/master/src), a nim proof-of-concept of Komodo's Decentralized Exchange (https://atomicdex.io/)

mratsim avatar Feb 05 '20 11:02 mratsim

I don't think false positives are false they always have a meaning in my experience.

Anyway, hmm, I was hoping nim did not get on the way anymore but I guess it's still the case. I was having issues even by just disabling the default allocator --useMalloc? or something.

sinkingsugar avatar Feb 05 '20 11:02 sinkingsugar

Ironically, some of nim's features hide issues that otherwise would be detected by valgrind / sanitizers: for example, because nim zero-inits everything, valgrind won't catch use of uninitialized fields because it cannot tell the difference between the developer intending to leave default in there and forgetting to set the field explicitly.

arnetheduck avatar Feb 05 '20 11:02 arnetheduck

image I didn't even enter nim territory with address sanitizer to fail on mac... Can't run address sanitizer on "bitcoin" code... guess that's a sanitizer issue tho probably.

sinkingsugar avatar Feb 07 '20 03:02 sinkingsugar

is that because asan requires registers for itself?

arnetheduck avatar Feb 07 '20 07:02 arnetheduck

Very curious yes, I have no idea. https://bugs.llvm.org/show_bug.cgi?id=21234 Good old Valgrind works tho 😄

Edit: likely just some inline asm issue?

sinkingsugar avatar Feb 07 '20 10:02 sinkingsugar