status-im / nimbus-eth1

Nimbus: an Ethereum Execution Client for Resource-Restricted Devices
https://status-im.github.io/nimbus-eth1/
Apache License 2.0
569 stars 116 forks source link

[Security] Fuzzing and various hardened/sanitized code checks #164

Open mratsim opened 6 years ago

mratsim commented 6 years ago

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:

arnetheduck commented 6 years ago

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

corpetty commented 5 years ago

Looking into this one.

lojikil commented 5 years ago

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.

Basically, what I recommend would be this:

lojikil commented 5 years ago

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.

kdeme commented 5 years ago

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.

0xc1c4da commented 4 years ago

➤ 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.

arnetheduck commented 4 years ago

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

kdeme commented 4 years ago

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.

sinkingsugar commented 4 years ago

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 :)

mratsim commented 4 years ago

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

sinkingsugar commented 4 years ago

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.

arnetheduck commented 4 years ago

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.

sinkingsugar commented 4 years ago

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.

arnetheduck commented 4 years ago

is that because asan requires registers for itself?

sinkingsugar commented 4 years ago

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?