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

Fuzz v6 #3914

Closed
wants to merge 3 commits into from
Closed

Fuzz v6 #3914

wants to merge 3 commits into from

Conversation

catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/2859

Describe changes:

  • Adds three fuzz targets
  • Change compilation scheme so as to compile fuzz targets (option --enable-fuzztargets for configure)

Modifies #3908 with needed rebase

I think that it will be useful to merge this code as continuous fuzzing will find more bugs than I did running these targets 10 minutes or less and finding already #3877 and #3890

What is left to be done before merging this ?
Are the following items relevant ? Is there anything else ?

  • using or removing fuzz_openFile
  • move PostConfLoadedSetup to init.c
  • load and reload yaml config file in fuzz_sigyamlpcap.c

Should we already start oss-fuzz integration on a specific branch ? cf https://github.com/catenacyber/oss-fuzz/blob/ac9d0d47951f7c884b33ca26920717a27b84abee/projects/suricata/Dockerfile#L31

For all modes : fuzz, unit tests, real suricata, etc...
And ways to compile them with enable-fuzztargets at configure time
Copy link
Member

@victorjulien victorjulien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README is gone too. Could you re-add some instructions on how to use it?

@@ -6,8 +6,11 @@ noinst_HEADERS = action-globals.h \
suricata-common.h threadvars.h tree.h \
util-validate.h
bin_PROGRAMS = suricata
if BUILD_FUZZTARGETS
bin_PROGRAMS += fuzz_applayerprotodetectgetproto fuzz_applayerparserparse fuzz_siginit fuzz_confyamlloadstring fuzz_decodepcapfile fuzz_sigyamlpcap
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you update this to have a ~80 char line limit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

suricata_LDADD = $(HTP_LDADD) $(RUST_LDADD)
suricata_LDADD = $(RUST_SURICATA_LIB) $(HTP_LDADD) $(RUST_LDADD)

fuzz_applayerprotodetectgetproto_SOURCES = tests/fuzz/fuzz_applayerprotodetectgetproto.c $(COMMON_SOURCES)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we unify these more? Its kind of ugly to have this list of similar blocks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent some hours looking for a solution to this. Do you have any idea/pointer ? For some macro-like function for Makefile.am files...

fuzz_applayerprotodetectgetproto_SOURCES = tests/fuzz/fuzz_applayerprotodetectgetproto.c $(COMMON_SOURCES)
fuzz_applayerprotodetectgetproto_LDFLAGS = $(all_libraries) ${SECLDFLAGS}
fuzz_applayerprotodetectgetproto_LDADD = $(RUST_SURICATA_LIB) $(HTP_LDADD) $(RUST_LDADD)
if HAVE_LIBFUZZINGENGINE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when would we not have this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For instance, CI does not need libFuzzingEngine to build the fuzz targets with the simple driver onefile.c
I use it as well to debug and reproduce crashes


if (alp_tctx == NULL) {
InitGlobal();
run_mode = RUNMODE_UNITTEST;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this set?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It made sense to me that fuzzing was some kind of testing.
What does that imply ?

suricata.delayed_detect = 1;

SupportFastPatternForSigMatchTypes();
//PostConfLoadedSetup(&suricata);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why isn't this used instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do it

@catenacyber catenacyber mentioned this pull request Jul 10, 2019
@catenacyber catenacyber deleted the fuzz-v6 branch September 4, 2020 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants