-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add more standalone fuzzing harnesses #64
base: master
Are you sure you want to change the base?
Conversation
Immediate comment - I'm mildly terrified about adding another 2000 files to this repo, but I don't know if that's FUD or not. |
For each harness there's a few "seed" cases with human-readable names, plus some more inputs obtained through fuzzing, with hash-like filenames. We can probably remove the latter and let oss-fuzz rediscover them, if you don't want all the files committed in the repository. |
I realise I didn't respond here - yes, please, if you could do that that would be great. |
That list is used by oss-fuzz, and probably somewhere else. Add the new altsvc, base64 and doh fuzzers to it.
This patch adds `-I` flag to compilation flags for standalone harnesses in Makefile.am. Variable CURLDIR is used to determine include path. This patch sets CURLDIR envvar in ossfuzz.sh, but name is taken from mainline.sh. That makes dependencies work with oss-fuzz. It should also make it work with mainline.sh
This fixes coverage collection for standalone harnesses.
Real target is function parsedate from parsedate.c The harness was written by Peter Goodman.
f43b34a
to
2987f5c
Compare
Basic test inputs for parse-date.
It does check if orginal string and unescaped data are same. Functions fuzzed: - curl_easy_escape - curl_easy_unescape
Simple inputs for escape fuzzer
Co-authored-by: Kelly Kaoudis <[email protected]>
This makes it easier to debug crashes.
2987f5c
to
e746887
Compare
|
||
extern "C" | ||
{ | ||
#define HAVE_STRUCT_TIMEVAL // HACK to let it compile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this hack still be in here?
|
||
/** | ||
* Fuzzing entry point. This function is passed a buffer containing a test | ||
* case. This test case should drive the CURL fnmatch function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not driving the fnmatch function.
*/ | ||
extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) | ||
{ | ||
std::string s(reinterpret_cast<const char*>(data), size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mildly worried that we could have a std::string buffer (which isn't null terminated) containing a buffer with a null in it, and what that would do for the fuzzing. Probably fine though.
(One of the things I've been wanting to get into the codebase is the FuzzedDataProvider which might help here, but equally might just do the same thing under the covers)
|
||
/** | ||
* Fuzzing entry point. This function is passed a buffer containing a test | ||
* case. This test case should drive the CURL fnmatch function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/fnmatch/base64/
#include <inttypes.h> | ||
#include <curl/curl.h> | ||
#include <assert.h> | ||
#define WARN_UNUSED_RESULT /* hack */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this hack still need to be in?
#include <lib/parsedate.h> | ||
} | ||
|
||
// fuzz_target.cc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??
@@ -0,0 +1,35 @@ | |||
extern "C" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These have lost the copyright header.
@@ -0,0 +1,18 @@ | |||
extern "C" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto copyright
|
||
extern "C" int LLVMFuzzerTestOneInput(char *data, size_t size) { | ||
time_t output = 0; | ||
char date[100]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few magic numbers here; might be best to define a constant MAX_DATE_LEN to be 100, then use that?
char date[MAX_DATE_LEN + 1];
size_t len = size > MAX_DATE_LEN ? MAX_DATE_LEN : size;
memcpy(date, data, len);
date[len] = 0;
@@ -31,8 +31,8 @@ shift $((OPTIND-1)) | |||
export CC=clang | |||
export CXX=clang++ | |||
FUZZ_FLAG="-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION" | |||
export CFLAGS="-fsanitize=address" | |||
export CXXFLAGS="-fsanitize=address -stdlib=libstdc++ $FUZZ_FLAG" | |||
export CFLAGS="-fsanitize=address,fuzzer-no-link" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain this, I'm unsure what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I recall, fuzzer-no-link
adds the instrumentation for fuzzing (like -fsanitize=fuzzer
would), but without linking libfuzzer in (so you can link it afterwards). When I first tried without this flag, the local mainline.sh builds were not running appropriately, but it might have been a matter of my environment and/or clang version. I can try again and see if it's really needed.
This PR adds some new standalone harnesses that fuzz:
Some of the harness code includes (from
CURLDIR
) or copies bits from internal headers; there might be a nicer way to do that.