Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update NSS project to use the build.sh and fuzzers provided by NSS #316

Merged
merged 1 commit into from
Jan 25, 2017
Merged

Update NSS project to use the build.sh and fuzzers provided by NSS #316

merged 1 commit into from
Jan 25, 2017

Conversation

ttaubert
Copy link
Contributor

The current fuzzers have been removed and are now covered by the new QuickDER target.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

The current fuzzers have been removed and are now covered by the new QuickDER target.
@googlebot
Copy link

CLAs look good, thanks!

@ttaubert
Copy link
Contributor Author

BTW, Here's our in-tree build script:

https://hg.mozilla.org/projects/nss/file/tip/automation/ossfuzz/build.sh

@kcc
Copy link
Contributor

kcc commented Jan 25, 2017

The change looks great. Just a quick check: did you test it in the OSS-Fuzz's docker environment?

@ttaubert
Copy link
Contributor Author

Yeah, both fuzzers that we currently have build and run just fine. crossing fingers

@kcc kcc merged commit 71b6b10 into google:master Jan 25, 2017
@kcc
Copy link
Contributor

kcc commented Jan 25, 2017

Excellent!
Keep adding more stuff there :)

@ttaubert
Copy link
Contributor Author

That's the plan ;) Thank you!

@kcc
Copy link
Contributor

kcc commented Jan 25, 2017

a question: are you using libFuzzer provided by OSS-Fuzz, or your own build of libFuzzer?

@ttaubert
Copy link
Contributor Author

We're using -lFuzzingEngine as stated in your documentation.

@kcc
Copy link
Contributor

kcc commented Jan 25, 2017

Perfect!

@ttaubert ttaubert deleted the update-nss branch January 25, 2017 21:33
@kcc
Copy link
Contributor

kcc commented Jan 25, 2017

What is -I$SRC/libfuzzer" for?

@kcc
Copy link
Contributor

kcc commented Jan 25, 2017

I also see "#include "FuzzerInternal.h"
Please don't, this is not supported :)
The fuzz targets should not include anything from libFuzzer because libFuzzer is not going to be the only engine used for these targets.

@ttaubert
Copy link
Contributor Author

Right... we probably don't need to include FuzzerInternal.h but rather FuzzerInterface.h. We need FuzzerRandom.h for custom mutators. How does OSS-Fuzz deal with targets that want/use custom mutators then?

@kcc
Copy link
Contributor

kcc commented Jan 25, 2017

OSS-Fuzz does not know about custom mutators.
If the binary builds and runs, ClusterFuzz will fuzz it.

It's ok to include FuzzerInterface.h (but not any other header), but remember that custom mutators
only work with libFuzzer today, and not with e.g. AFL. (which is fine, so just FYI).
But I actually tried to make the interface surface so thin that you don't really need to include
FuzzerInterface.h -- you may just declare the 1 or 2 functions you need from there.

And, FuzzerInterface.h is still experimental to some extent, be prepared for some changes in future.

@ttaubert
Copy link
Contributor Author

I see, okay. It's fine to not have custom mutators with AFL, and we can react to API changes quickly.

So we should probably not user fuzzer::Random() but feed the seed into std::mt19937 or our own RNG?

@kcc
Copy link
Contributor

kcc commented Jan 25, 2017

So we should probably not user fuzzer::Random() but feed the seed into std::mt19937 or our own RNG?

Yes, that's the idea.
You just take the seed and use it with any pseudo RNG.
(It's important to not use a real RNG that will produce different results from run to run even with the same seed)

@ttaubert
Copy link
Contributor Author

Makes sense. We'll fix and land this tomorrow. I'll let you know!

@ttaubert
Copy link
Contributor Author

@kcc
Copy link
Contributor

kcc commented Jan 26, 2017

Currently the build is marked as unstable, i.e. the built binaries don't run.
Looking here: https://oss-fuzz-build-logs.storage.googleapis.com/status.html
(then here: https://oss-fuzz-build-logs.storage.googleapis.com/build_logs/nss/latest.txt)

testing hash
Using seed corpus: hash_seed_corpus.zip
hash -rss_limit_mb=2048 -timeout=25 -runs=32 /tmp/seed_corpus/
bash: line 0: hash: -s: invalid option
hash: usage: hash [-lr] [-p pathname] [-dt] [name ...]
ERROR: bad exit code: 2

this is clearly a bug in our infra which calls the fuzzer binaries w.o a path,
and the name of your binary collides with a bash builtin.

We need to fix it, of course.

@kcc
Copy link
Contributor

kcc commented Jan 27, 2017

.. and fixed now

@inferno-chromium
Copy link
Collaborator

The major bug right now is fuzzer is continuously crashing on this error. https://clusterfuzz-external.appspot.com/v2/testcase-detail/4744595573309440 will take a while to open since it has this error all over.

hash: ../../fuzz/shared.h:21: NSSDatabase::NSSDatabase(): Assertion `NSS_NoDB_Init(nullptr) == SECSuccess failed.
ASAN:DEADLYSIGNAL

==1==ERROR: AddressSanitizer: ABRT on unknown address 0x000000000001 (pc 0x7f8fb75f9418 bp 0x000000c06760 sp 0x7ffcca718ea8 T0)
SCARINESS: 10 (signal)
#0 0x7f8fb75f9417 in gsignal
#1 0x7f8fb75fb019 in abort
#2 0x7f8fb75f1bd6 in libc.so.6
#3 0x7f8fb75f1c81 in __assert_fail
#4 0x51502a in NSSDatabase::NSSDatabase() /src/nss/fuzz/shared.h:21:19
#5 0x51502a in LLVMFuzzerTestOneInput /src/nss/fuzz/hash_target.cc:19
#6 0x969ee8 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:550:13
#7 0x96906d in fuzzer::Fuzzer::ShuffleAndMinimize(std::__1::vector<std::__1::vector<unsigned char, std::__1::allocator >, std::__1::allocator<std::__1::vector<unsigned char, std::__1::allocator > > >) /src/libfuzzer/FuzzerLoop.cpp:477:3
#8 0xa4790a in fuzzer::FuzzerDriver(int
, char***, int ()(unsigned char const, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:565:6
#9 0x9c69b8 in main /src/libfuzzer/FuzzerMain.cpp:20:10
#10 0x7f8fb75e482f in __libc_start_main
#11 0x41dc68 in _start
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: ABRT (/lib/x86_64-linux-gnu/libc.so.6+0x35417)
==1==ABORTING

DavidKorczynski added a commit that referenced this pull request Jul 9, 2024
During development it's convenient to being able to adjust this, for
example when focus during development is on aspects of OSS-Fuzz-gen that
is unrelated to code fixing it's convenient to decrease this to speed up
evaluation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants