Add a bunch of NSS ASN.1 fuzzers

Add a bunch of NSS ASN.1 fuzzers

BUG=none

[kcc2, 2016-02-06 05:36:03]

kcc@chromium.org changed reviewers:
+ kcc@chromium.org

[kcc2, 2016-02-06 05:36:03]

Cool! 
While we are figuring out where to get instrumented libnss & friends, I'd like
to try running these targets manually... 

May I ask you to ensure that they compile? 

asn1_bmpstring_fuzzer.cc:16:38: error: cannot initialize a member subobject of
type 'unsigned int' with an rvalue of type 'nullptr_t'
  SECItem quick_dest = {siBuffer, 0, nullptr};

[kcc2, 2016-02-06 05:37:39]

On 2016/02/06 05:36:03, kcc2 wrote:
> Cool! 
> While we are figuring out where to get instrumented libnss & friends, I'd like
> to try running these targets manually... 
> 
> May I ask you to ensure that they compile? 
> 
> asn1_bmpstring_fuzzer.cc:16:38: error: cannot initialize a member subobject of
> type 'unsigned int' with an rvalue of type 'nullptr_t'
>   SECItem quick_dest = {siBuffer, 0, nullptr};

and let's start with just one (your choice)

[Ryan Sleevi, 2016-02-08 00:01:17]

All should be compiling now :)

There is a BUILD.gn for NSS, but it was only tested on iOS, and would likely
have issues.

On the NSS side, https://bugzilla.mozilla.org/show_bug.cgi?id=1233568 is the bug
for integrating ASAN/MSAN/UBSAN builds directly into the build system.

[Ryan Sleevi, 2016-02-10 00:30:56]

kcc: This is ready for your review

[kcc2, 2016-02-10 00:46:06]

On 2016/02/10 00:30:56, Ryan Sleevi wrote:
> kcc: This is ready for your review

question about  asn1_bitstring_fuzzer.cc: why is it spending 91% of time in
memset? 
Is it a problem of the API being tested, or it can be fixed in the target
function somehow? 

This is the profile I see now: 
-  92.11%  F-asn1_bitstrin  libc-2.19.so             [.] memset                 
                                                                                
                                 ▒
   - memset                                                                     
                                                                                
                                 ▒
      - 55.72% ClearArenaList                                                   
                                                                                
                                 ◆
         - 52.54% FreeArenaList                                                 
                                                                                
                                 ▒
            - 89.69% PL_FreeArenaPool                                           
                                                                                
                                 ▒
               - PORT_FreeArena_Util                                            
                                                                                
                                 ▒
                  - 78.65% SEC_ASN1DecoderFinish_Util                           
                                                                                
                                 ▒
                       SEC_ASN1Decode_Util                                      
                                                                                
                                 ▒
                       SEC_ASN1DecodeItem_Util

[Ryan Sleevi, 2016-02-10 00:50:10]

On 2016/02/10 00:46:06, kcc2 wrote:
> On 2016/02/10 00:30:56, Ryan Sleevi wrote:
> > kcc: This is ready for your review
> 
> question about  asn1_bitstring_fuzzer.cc: why is it spending 91% of time in
> memset? 
> Is it a problem of the API being tested, or it can be fixed in the target
> function somehow? 

NSS_DISABLE_ARENA_FREE_LIST=1 when running the target (same as we do for
valgrind)

That said, it looks like SEC_ASN1DecoderFinish is always set to call
PORT_FreeArena with zeroit set -
http://mxr.mozilla.org/nss/source/lib/util/secasn1d.c#2967 , so faster iteration
would need to change that to PR_FALSE for testing (this is a "legacy" API, but
still quite exposed, so unlikely that upstream will accept any changes to it)

[kcc2, 2016-02-10 02:25:23]

https://codereview.chromium.org/1677803002/diff/120001/testing/libfuzzer/fuzz...
File testing/libfuzzer/fuzzers/nss/asn1_bitstring_fuzzer.cc (right):

https://codereview.chromium.org/1677803002/diff/120001/testing/libfuzzer/fuzz...
testing/libfuzzer/fuzzers/nss/asn1_bitstring_fuzzer.cc:20: // Attempt the
QuickDER path.
does this have to be a single target, or it can be split into two? 
(quick and legacy). 
My fear is that if one of the parts is very slow we will not be able to properly
test the other part. 

Also, what will happen if we make quick_arena/legacy_arena globals and never
free them?
Will there be a permanent leak?

[Ryan Sleevi, 2016-02-10 02:45:36]

https://codereview.chromium.org/1677803002/diff/120001/testing/libfuzzer/fuzz...
File testing/libfuzzer/fuzzers/nss/asn1_bitstring_fuzzer.cc (right):

https://codereview.chromium.org/1677803002/diff/120001/testing/libfuzzer/fuzz...
testing/libfuzzer/fuzzers/nss/asn1_bitstring_fuzzer.cc:20: // Attempt the
QuickDER path.
On 2016/02/10 02:25:22, kcc2 wrote:
> does this have to be a single target, or it can be split into two? 
> (quick and legacy). 
> My fear is that if one of the parts is very slow we will not be able to
properly
> test the other part. 

Yes, we totally can.

Templated logic is totally fine, right? If so, considering that the permutations
of all of these tests are "template, type to receive template, initialization
code for type to receive template", the bulk of the tests could be de-duplicated
into a common templated base.

> Also, what will happen if we make quick_arena/legacy_arena globals and never
> free them?
> Will there be a permanent leak? 

We can use the arena markers for a test

That is:

void LLVMFuzzerTestOneInput(...) {
  void* mark = PORT_ArenaMark(arena);
  // do stuff that allocates in the arena
  PORT_ArenaRelease(arena, mark);
}

That will just mark the arena as shrunk, but will still memset.

To eliminate the memsets, we would need to undefine DEBUG (c.f.
http://mxr.mozilla.org/nspr/source/lib/ds/plarena.h#184 )

[aizatsky, 2016-02-11 01:07:39]

Description was changed from

==========
Add a bunch of NSS ASN.1 fuzzers

BUG=none
==========

to

==========
Add a bunch of NSS ASN.1 fuzzers

BUG=none
==========

[aizatsky, 2016-02-11 01:07:39]

aizatsky@chromium.org changed reviewers:
+ aizatsky@chromium.org

[kcc2, 2016-02-11 05:28:20]

> Templated logic is totally fine, right? 

Yes, templates would be welcome here. 

From an offline discussion looks like speeding up these targets 
should probably be done outside of this code (either by using the opt build, or
by changing nss code)

[Ryan Sleevi, 2016-03-03 02:21:19]

Upstream bugs:

NSS:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1253101
- https://bugzilla.mozilla.org/show_bug.cgi?id=1253107

NSPR:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1253105

1253101 is the necessary one for ClusterFuzz, and that should land soon (NSS is
frozen-ish while branches are sorted out). It's a minor patch if we wanted to
carry it internally.

1253107 is a one-line fix to change the call to PR_FALSE, if we wanted to carry
that locally until upstream can sort out the best path.

1263105 remains TBD, but there's a two-line patch if we wanted to carry it
locally.

In the mean-time, I converted to a common templated base and fixed a few bugs. S

[mmoroz, 2016-03-03 10:28:31]

mmoroz@chromium.org changed reviewers:
+ mmoroz@chromium.org

[mmoroz, 2016-03-03 10:28:33]

https://codereview.chromium.org/1677803002/diff/180001/testing/libfuzzer/fuzz...
File testing/libfuzzer/fuzzers/nss/BUILD.gn (right):

https://codereview.chromium.org/1677803002/diff/180001/testing/libfuzzer/fuzz...
testing/libfuzzer/fuzzers/nss/BUILD.gn:15:
fuzzer_test("asn1_algorithmid_fuzzer") {
Do you mind do add "nss_" prefix for all these fuzzers? As we are getting more
and more fuzzers, detailed names are very useful. For example, a one will be
able to list only nss fuzzers at coverage dashboard using "nss" substring as a
filter.

For other fuzzers we also tend to use the following name format:
<module_name>_<fuzzer_name>_fuzzer

May be in future we should create another structure (use subdirectories or
anything else) to organize lot of fuzzers, but right now we are doing this way.

[mmoroz, 2016-11-07 13:54:32]

YI, we are running these fuzzers using oss-fuzz project infrastructure:
https://github.com/google/oss-fuzz/tree/master/targets/nss

Any improvements are more than welcome :)