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

Simplify launching and testing fuzzing tools #366

Merged
merged 6 commits into from
Feb 6, 2019

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Feb 5, 2019

Miscellaneous minor tweaks and annoyance fixes in fuzzing tools that I found useful after testing and actually using fuzzing on Linux and macOS.

Use command-line arguments instead of pipes

stdin is a bit easier to use from the programming viewpoint, but it complicates debugging crashes. It is simpler to pass the input file as a command-line argument than to setup file redirection with lldb or gdb.

Teach all tools to read input from the first command-line argument, update AFL command-line to pass the input via arguments, and update readme to prefer this form.

This gives us easier time with debugger:

gdb --args build/afl/${tool} tools/afl/input/${tool}/some_input.dat

and then we can just "run" in gdb's command line, instead of having to setup redirection every time.

Launch trap as a separate command

We should setup signal trap as a separate shell command for it to have effect on the next one. The previous form works on macOS for some weird reason but it does not work on Linux.

Organize fuzzer output into subdirectories

Separate tool name in filesystem hierarchy makes it easier to iterate over the findings for later reporting.

Link statically against Themis

Dynamic linkage with non-system installation paths is a minefield. For example, it works incorrectly on macOS if Themis is already installed to /usr/local/lib (that's because of the "installation path" thingy in dylibs). On Linux dynamic linkage requires LD_LIBRARY_PATH to be correctly configured even when building the fuzzing tools.

Use static linkage instead. It turned out to be not that hard. We just have to add libsoter.a explicitly as well as LDFLAGS from outer Themis build to pull in the OpenSSL/LibreSSL/BoringSSL dependency.

stdin is a bit easier to use from the programming viewpoint, but it
complicates debugging crashes. It is simpler to pass the input file
as a command-line argument than to setup file redirection with lldb
or gdb.

Teach all tools to read input from the first command-line argument,
update AFL command-line to pass the input via arguments, and update
readme to prefer this form.

This gives us easier time with debugger:

    gdb --args build/afl/${tool} tools/afl/input/${tool}/*

and then we can just "run" in gdb's command line, instead of having
to setup redirection every time.
We should setup signal trap as a separate shell command for it to have
effect on the next one. The previous form works on macOS for some weird
reason but it does not work on Linux.
Separate tool name in filesystem hierarchy makes it easier to iterate
over the findings for later reporting.
Dynamic linkage with non-system installation paths is a minefield.
For example, it works incorrectly on macOS if Themis is already
installed to /usr/local/lib (that's because of the "installation path"
thingy in dylibs). On Linux dynamic linkage requires LD_LIBRARY_PATH
to be correctly configured even when building the fuzzing tools.

Use static linkage instead. It turned out to be not that hard. We just
have to add libsoter.a explicitly as well as LDFLAGS from outer Themis
build to pull in the OpenSSL/LibreSSL/BoringSSL dependency.
@ilammy ilammy added core Themis Core written in C, its packages infrastructure Automated building and packaging labels Feb 5, 2019
@vixentael
Copy link
Contributor

A bit weird about static vs dynamic linkage, I was trying on mac and haven't experience any issues, but I'm glad that you fixed that!

FILE* input = fopen(argv[1], "rb");
if (!input)
{
return 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about to print some message that we expect filepath in one required argument to avoid the need to dive into sources to understand why exit status 1?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TBH, the exit statuses are meaningless now. AFL ignores them completely, it's in only for the crashes. I hoped that it might discriminate between different exit codes, but nope.

You're right. I guess I'll add some messages to stderr when we exit with non-zero status. This should make it easier for humans with the tools.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Lagovas here you go. Now the tools print a "usage" line if the command-line is incorrect, as well as some comments on why exactly we're exiting with non-zero status or aborting.

It's not nice to humans just return exit codes. We should at least give
them a hint if the command-line is incorrect. Add some proper error
messages as well for cases when we can't read the input file, or some
Themis function breaks, or a really important test fails.
@ilammy ilammy merged commit 069f07d into cossacklabs:master Feb 6, 2019
@ilammy ilammy deleted the fuzzer-tweaks branch February 9, 2019 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Themis Core written in C, its packages infrastructure Automated building and packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants