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

aflplusplus: init at 2.59c #76645

Merged
merged 1 commit into from Apr 23, 2020
Merged

aflplusplus: init at 2.59c #76645

merged 1 commit into from Apr 23, 2020

Conversation

risicle
Copy link
Contributor

@risicle risicle commented Dec 29, 2019

Motivation for this change

https://github.com/vanhauser-thc/AFLplusplus

AFL++ is a heavily enhanced version of AFL, incorporating many features and improvements from the community.

Package is based on the original AFL packaging, but I've tried to enable many of the optional extras.

cc @thoughtpolice

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@risicle
Copy link
Contributor Author

risicle commented Jan 1, 2020

As noted in #76762, I should be able to enable unicorn mode, but only after unicorn 1.0.2 is released and makes it to nixpkgs.

@risicle
Copy link
Contributor Author

risicle commented Jan 2, 2020

Added wine support for i686 builds (default off - huge closure addition).

@GrahamcOfBorg build aflplusplus

@risicle
Copy link
Contributor Author

risicle commented Apr 18, 2020

Rebased, added afl-clang-fast++ wrapper fix.

@GrahamcOfBorg build aflplusplus

# nix's cc wrapper
rm $out/bin/afl-clang-fast++
cp $out/bin/afl-clang-fast $out/bin/afl-clang-fast++
for x in $out/bin/afl-clang-fast $out/bin/afl-clang-fast++; do
Copy link
Contributor

Choose a reason for hiding this comment

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

For 2.64c the wrapping doesn't seem to be needed anymore, as far as I've seen.

This issue resolves it: AFLplusplus/AFLplusplus#316

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh that's cool.

# first though we need to replace the afl-clang-fast++ symlink with
# a real copy to prevent wrapProgram skipping the symlink and confusing
# nix's cc wrapper
rm $out/bin/afl-clang-fast++
Copy link
Contributor

Choose a reason for hiding this comment

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

And consequently, if there's no need for wrapping anymore, there's no need for this hack either.

AFLplusplus/AFLplusplus#318, I made a PR to AFLplusplus to fail instead of silently starting CC in that case as well :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice

--run 'export AFL_CC=''${AFL_CC:-${clang}/bin/clang} AFL_CXX=''${AFL_CXX:-${clang}/bin/clang++}'
done
# do similar for afl-gcc and afl-gcc-fast
for x in $out/bin/afl-gcc $out/bin/afl-gcc-fast; do
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you check if afl-gcc works without wrapping? I thought it did.

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 think I wouldn't have added it if I didn't find it necessary - perhaps it's not needed for 2.64 though?

@@ -585,6 +585,12 @@ in
stdenv = clangStdenv;
};

aflplusplus = callPackage ../tools/security/aflplusplus {
stdenv = clangStdenv;
python = python37;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why pin to 3.7? Is there a reason for that?

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'm having to do a bit of archaeology to figure this out, but the readme for the python mutator at the time made specific mention of python 3.7: https://github.com/AFLplusplus/AFLplusplus/blob/939721e2cbcf14f0f9577575c09e16be2ced25fe/python_mutators/README#L4 so I must have assumed some version sensitivity.

@risicle
Copy link
Contributor Author

risicle commented Apr 22, 2020

@Mindavi these are all good suggestions - if you test this package as it is, and it works for you, I'll merge this and then review your additions and bump to 2.64c (also add yourself as co-maintainer!). I'd like to get this merged first because I've been using it solidly for months and know it's solid.

@Mindavi
Copy link
Contributor

Mindavi commented Apr 22, 2020

Sounds good! I'll test this out as soon as I have time, and then I hope it's good to go. I understand you'd like a well tested version, the new release is just fresh.

@Mindavi
Copy link
Contributor

Mindavi commented Apr 23, 2020

Builds fine on my machine. Just started a testrun, but I'm not expecting much issues.

One issue though, it seems like afl-clang++ is not wrapped correctly (try compiling a C++ binary, it will error with missing headers). Not sure about afl-clang, but it probably falls back (as shown earlier in the PR I made against aflplusplus) to the C compiler. Just started a run and aflplusplus seems to run stable with a binary compiled with afl-clang-fast++.

So if that wrapping issue is fixed, I think this can be merged.

@risicle
Copy link
Contributor Author

risicle commented Apr 23, 2020

Oh - that's exactly the issue which my "copy, don't link" change was supposed to fix. What's happening is the nix cc wrapper isn't detecting that it should run in c++ mode and so isn't including the cpp stdlib (I never experienced the crash you mentioned in AFLplusplus/AFLplusplus#316). I have found that c++ projects are harder to build with afl than c ones, especially ones that use a mix of c/c++. What method of setting the compiler were you using when you tried this?

An example of how I successfully afl-compiled a nix c++ package was adding the lines

   AFL_INST_RATIO="10";
   AFL_HARDEN="1";
   AFL_LLVM_LAF_SPLIT_SWITCHES="1";
   AFL_LLVM_LAF_TRANSFORM_COMPARES="1";
   AFL_LLVM_LAF_SPLIT_COMPARES="1";
   preConfigure = ''
     export CC=${aflplusplus}/bin/afl-clang-fast CXX=${aflplusplus}/bin/afl-clang-fast++
   '';

to the qpdf package. Worked a treat for me. Does it work for you?

@Mindavi
Copy link
Contributor

Mindavi commented Apr 23, 2020

Probably related to the patch shown in AFLplusplus/AFLplusplus#318. It's not actually the nix cc wrapper as far as I can tell, rather aflplusplus that silently falls back to the clang c compiler. I guess you missed adding the trick for the afl-clang++ compiler.

My test case is simple:

#include <new>
#include <iostream>

int main() {
  std::cout << "Hi!\n";
  return 0;
}

Compiled with afl-clang++ test.cpp, which fails with 'header <new> not found'.

FYI: afl-clang-fast++ works correctly

@risicle
Copy link
Contributor Author

risicle commented Apr 23, 2020

Oh, sorry, yes, I wasn't reading properly. afl-clang++ is neglected. Let me see about that.

@risicle
Copy link
Contributor Author

risicle commented Apr 23, 2020

Ok, I don't seem to be able to fix it in the same way for some reason. Let's sort it out in the 2.64 bump.

@risicle risicle merged commit 324dd67 into NixOS:master Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants