various fixes and changes

5 files changed, 54 insertions, 86 deletions

diff --git a/CHANGES b/CHANGES
index 5f6d4b1..9b94b79 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,19 +2,22 @@ Changelog
 =========
 
 https://github.com/vanhauser-thc/afl-dyninst
- - First fix for -l option, did copy and instrument all libs
+ - performance level 1 is now standard and only -x needed for extra performance
+   flags. There was no disadvantage in previous level 1 so its default now.
+ - removed -l option because of bugs in the implementation, bugs in dyninst
+   and behavior of dyninst that was not good for our purpose. Use -r instead
  - Only compile dyninst9 bug workaround when necessary
  - added -I option (only instrument specific functions)
- - updated the README for guidance to build against dyninst version 10
+ - updated the README for guidance to build with dyninst version 10
  - added support for dyninst version 10
- - added -x performance optimization options, before this afl-dyninst was meh,
-   now it is OK. It supports 3 levels: -x (+45%, -xx additional +45%,
-    -xxx additional ~3% but so far only on intel x64)
+ - added -x performance optimization option and a speed enhancement patch that
+   is always active. before this afl-dyninst was meh, now it is OK.
    top speed is now ~110% on stock afl-dyninst and ~65% on stock afl-gcc
  - -e option now also understands function names, not only 0x1234 addresses
  - searches for multiple entrypoints now: main, init, start and _NAME variants
  - afl-dyninst now works fine with AARCH64 and PPC :)
  - more verbose output, -vv -vvv support
+ => at this stage did talos-vulndev/afl-dyninst the last pull from this repo
  - fixed some typos
  - renamed afl-fuzz.sh to afl-fuzz-dyninst.sh and make install
    installs now the scripts
@@ -33,4 +36,4 @@ https://github.com/vanhauser-thc/afl-dyninst
  - code beautifications, more detailed output, spelling fixes
  - added afl-dyninst.sh and afl-fuzz.sh helper scripts which presets
    the necessary environment variables
- - based on https://github.com/talos-vulndev/afl-dyninst
+ => intial fork from https://github.com/talos-vulndev/afl-dyninst
diff --git a/Makefile b/Makefile
index d029936..305e0a1 100644
--- a/Makefile
+++ b/Makefile
@@ -31,11 +31,7 @@ CFLAGS = -Wall -O3 -g -std=gnu99
 all: afl-dyninst libAflDyninst.so
 
 afl-dyninst:	afl-dyninst.o
-	$(CXX) $(CXXFLAGS) -L$(DYNINST_ROOT)/lib \
-		-L$(DEPS_ROOT)/lib \
-		-o afl-dyninst afl-dyninst.o \
-		$(DYNINST_OPT) \
-		-ldyninstAPI
+	$(CXX) $(CXXFLAGS) -L$(DYNINST_ROOT)/lib -L$(DEPS_ROOT)/lib -o afl-dyninst afl-dyninst.o $(DYNINST_OPT) -ldyninstAPI
 
 libAflDyninst.so: libAflDyninst.cpp
 	$(CXX) -O3 -std=c++11 $(LIBFLAGS) -I$(AFL_ROOT) -I$(DYNINST_ROOT)/include -I$(DEPS_ROOT)/include libAflDyninst.cpp -o libAflDyninst.so
diff --git a/README.md b/README.md
index 82ede34..3f71c19 100644
--- a/README.md
+++ b/README.md
@@ -66,8 +66,6 @@ Depending on the age of your Linux OS you can try to use packages from your dist
 Usage: afl-dyninst -dfvxD -i binary -o  binary -l library -e address -E address -s number -S funcname -I funcname -m size
    -i: input binary program
    -o: output binary program
-   -d: do not instrument the binary, only supplied libraries
-   -l: linked library to instrument (repeat for more than one)
    -r: runtime library to instrument (path to, repeat for more than one)
    -e: entry point address to patch (required for stripped binaries)
    -E: exit point - force exit(0) at this address (repeat for more than one)
@@ -77,18 +75,11 @@ Usage: afl-dyninst -dfvxD -i binary -o  binary -l library -e address -E address
    -I: only instrument this function and nothing else (repeat for more than one)
    -S: do not instrument this function (repeat for more than one)
    -D: instrument only a simple fork server and also forced exit functions
-   -x: experimental performance modes (can be set up to two times)
-         -x (level 1):  ~40-50%% improvement
-         -xx (level 2): ~100%% vs normal, ~40%% vs level 1
+   -x: experimental performance mode (~25-50% speed improvement)
    -v: verbose output
+   Note: options -l and -d have been deprecated, use -r instead.
 ```
 
-Switch -l is used to supply the names of the libraries that should 
-be instrumented along the binary. Instrumented libraries will be copied
-to the current working directory. This option can be repeated as many times
-as needed. Depending on the environment, the LD_LIBRARY_PATH should be set 
-to point to instrumented libraries while fuzzing. 
-
 Switch -e is used to manualy specify the entry point where initialization
 callback is to be inserted. For unstipped binaries, afl-dyninst defaults 
 to using _init of the binary as an entry point. In case of stripped binaries
@@ -122,15 +113,23 @@ when the register is used for function parameters.
 
 Switch -S allows you to not instrument specific functions.
 This options is mainly to hunt down bugs in dyninst.
+Can be specified multiple times.
+
+Switch -I specified to only instrument specific functions.
+This option is amazing with large and threaded targets.
+Can be specified multiple times.
 
 Switch -D installs the afl fork server and forced exit functions but no
-basic block instrumentation. That would serve no purpose - unless there is
-another interesting tool coming up: afl-pin (already available at
-https://github.com/vanhauser-thc/afl-pin) and afl-dynamorio (wip)
+basic block instrumentation. That would serve no purpose - unless there are
+other tools that need that: 
+ * [https://github.com/vanhauser-thc/afl-dynamorio](https://github.com/vanhauser-thc/afl-dynamorio)
+ * [https://github.com/vanhauser-thc/afl-pin](https://github.com/vanhauser-thc/afl-pin)
+
+Switch -x enables an experimental performance mode (+25-50% speed). Just try it
+and if the target crashes too often, instrument again without this. Should not
+crash though.
 
-Switch -x enables performance modes, -x is level 1 and -xx is level 2.
-level 1 (-x) is highly recommended (+50%).
-level 2 (-xx) gives an additonal 40% but removes (usually unnecessary) precautions
+Note that the -l and -d options have been deprecated. Use -r instead.
 
 
 ## Example of instrumenting a target binary
@@ -140,7 +139,7 @@ of libdyninstAPI_RT.so.
 
 ```
 $ export DYNINSTAPI_RT_LIB=/usr/local/lib/libdyninstAPI_RT.so
-$ ./afl-dyninst -i ./unrar -o ./rar_ins -e 0x4034c0 -s 10
+$ ./afl-dyninst -i ./unrar -o ./rar_ins -e 0x4034c0 -x
 Skipping library: libAflDyninst.so
 Instrumenting module: DEFAULT_MODULE
 Inserting init callback.
@@ -155,7 +154,7 @@ and outputing to unrar_ins
 You can also use the afl-dyninst.sh helper script which sets the required
 environment variables for you:
 ```
-$ ./afl-dyninst.sh -i ./unrar -o ./rar_ins -e 0x4034c0 -s 10
+$ ./afl-dyninst.sh -i ./unrar -o ./rar_ins -e 0x4034c0 -x
 ```
 
 
@@ -191,14 +190,14 @@ Problem 1: The binary does not work (crashes or hangs)
 
 Solution: increase the -m parameter. -m 8 is the minimum recommended, on some
           targets -m 16 is required etc.
-          You can also try to remove -x performance enhancers
+          You can also try to remove the -x performance enhancer
 
 
 Problem 2: Basically every fuzzing test case is reported as crash although it
            does not when running it from the command line
 
 Solution: This happens if the target is using throw/catch, and dyninst's
-          modification result in that the cought exception is not resetted and
+          modification result in that the caught exception is not resetted and
           hence abort() is triggered.
           No solution to this issue is known yet.
           Binary editing the target binary to perform _exit(0) would help though.
diff --git a/afl-dyninst.cpp b/afl-dyninst.cpp
index 01c5c5e..dcaeed0 100644
--- a/afl-dyninst.cpp
+++ b/afl-dyninst.cpp
@@ -42,7 +42,7 @@ set<string> skipAddresses;
 set<string> onlyAddresses;
 set<unsigned long> exitAddresses;
 unsigned int bbMinSize = 10;
-int bbSkip = 0, performance = 0;
+int bbSkip = 0, performance = 1;
 bool skipMainModule = false, do_bb = true, dynfix = false;
 unsigned long int insertions = 0;
 uintptr_t mapaddr = 0;
@@ -55,11 +55,9 @@ const char *functions[] = {"main", "_main", "_initproc", "_init", "start", "_sta
 const char *instLibrary = "libAflDyninst.so";
 
 static const char *OPT_STR = "fi:o:l:e:E:vs:dr:m:S:I:Dx";
-static const char *USAGE = " -dfvxD -i <binary> -o <binary> -l <library> -e <address> -E <address> -s <number> -S <funcname> -I <funcname> -m <size>\n \
+static const char *USAGE = " -fvxD -i <binary> -o <binary> -e <address> -E <address> -s <number> -S <funcname> -I <funcname> -m <size>\n \
   -i: input binary \n \
   -o: output binary\n \
-  -d: do not instrument the binary, only supplied libraries\n \
-  -l: linked library to instrument (repeat for more than one)\n \
   -r: runtime library to instrument (path to, repeat for more than one)\n \
   -e: entry point address to patch (required for stripped binaries)\n \
   -E: exit point - force exit(0) at this address (repeat for more than one)\n \
@@ -69,10 +67,9 @@ static const char *USAGE = " -dfvxD -i <binary> -o <binary> -l <library> -e <add
   -I: only instrument this function and nothing else (repeat for more than one)\n \
   -S: do not instrument this function (repeat for more than one)\n \
   -D: instrument only a simple fork server and also forced exit functions\n \
-  -x: experimental performance modes (can be set up to two times)\n \
-        -x (level 1):  ~40-50%% improvement\n \
-        -xx (level 2): ~100%% vs normal, ~40%% vs level 1\n \
-  -v: verbose output\n";
+  -x: experimental performance mode (~25-50% speed improvement)\n \
+  -v: verbose output\n \
+  Note: options -l and -d have been deprecated, use -r instead.\n";
 
 bool parseOptions(int argc, char **argv) {
   int c;
@@ -90,10 +87,7 @@ bool parseOptions(int argc, char **argv) {
               performance = 2;
       #endif
             } else*/
-      if (performance > 2) {
-        fprintf(stderr, "Warning: maximum performance level is 2\n");
-        performance = 2;
-      }
+      if (performance > 2) performance = 2;
       break;
     case 'I':
       onlyAddresses.insert(optarg);
@@ -113,8 +107,9 @@ bool parseOptions(int argc, char **argv) {
       instrumentedBinary = optarg;
       break;
     case 'l':
-      instrumentLibraries.insert(optarg);
-      break;
+       fprintf(stderr, "Error: option -l has been removed due implementation issues, dyninst behaviour and dyninst bugs. Please use -r.\n");
+       exit(-1);
+       break;
     case 'E':
       exitAddresses.insert(strtoul(optarg, NULL, 16));
       break;
@@ -128,12 +123,15 @@ bool parseOptions(int argc, char **argv) {
       bbMinSize = atoi(optarg);
       break;
     case 'd':
-      skipMainModule = true;
+      //skipMainModule = true;
+      fprintf(stderr, "Warning: option -d has been deprecated, due various issues. Just ignore -o file :-)\n");
       break;
     case 'f':
 #if (__amd64__ || __x86_64__)
+#if (DYNINST_MAJOR_VERSION < 10)
       dynfix = true;
 #endif
+#endif
       break;
     case 'D':
       do_bb = false;
@@ -147,11 +145,6 @@ bool parseOptions(int argc, char **argv) {
     }
   }
 
-  if (performance > 0 && do_bb == false) {
-    cerr << "Warning: -x performance options only enhance basic block coverage, not forkserver only mode" << endl;
-    performance = 0;
-  }
-
   if (originalBinary == NULL) {
     cerr << "Input binary is required!" << endl;
     cerr << "Usage: " << argv[0] << USAGE;
@@ -164,12 +157,6 @@ bool parseOptions(int argc, char **argv) {
     return false;
   }
 
-  if (skipMainModule && instrumentLibraries.empty()) {
-    cerr << "If using option -d , option -l is required." << endl;
-    cerr << "Usage: " << argv[0] << USAGE;
-    return false;
-  }
-
   return true;
 }
 
@@ -198,6 +185,8 @@ bool insertCallToInit(BPatch_addressSpace *appBin, BPatch_function *instIncFunc,
     return false;
   }
 
+  // THIS BLOCK IS DISABLED - dyninst is too volatile for this to work reliably
+  // disabled because performance can not be greater than 2
   if (performance >= 3 && install_hack == true) {
     cout << "Inserting global variables" << endl;
     // we set up a fake map so we do not have crashes if the the forkserver
@@ -326,6 +315,7 @@ bool insertBBCallback(BPatch_addressSpace *appBin, BPatch_function *curFunc, cha
 #endif
       BPatch_funcCallExpr instIncExpr(*instBBIncFunc, instArgs);
 
+
 #if (DYNINST_MAJOR_VERSION < 10)
       if (dynfix == true)
         handle = appBin->insertSnippet(instIncExpr1, *bbEntry, BPatch_callBefore, BPatch_firstSnippet);
@@ -468,9 +458,9 @@ int main(int argc, char **argv) {
 
   if (!initAflForkServer || !bbCallback || !forceCleanExit
 #if (DYNINST_MAJOR_VERSION < 10)
-      || !save_rdi || !restore_rdi
+      || !save_rdi || !restore_rdi 
 #endif
-  ) {
+     ) {
     cerr << "Instrumentation library lacks callbacks!" << endl;
     return EXIT_FAILURE;
   }
@@ -479,6 +469,7 @@ int main(int argc, char **argv) {
 
   // if an entrypoint was set then find function, else find _init
   BPatch_function *funcToPatch = NULL;
+
   if (entryPoint == 0 && entryPointName == NULL) {
     if (func2patch == NULL) {
       cerr << "Couldn't locate _init, specify entry point manually with -e 0xaddr" << endl;
@@ -547,39 +538,20 @@ int main(int argc, char **argv) {
     return EXIT_FAILURE;
   }
 
-  bool skip_until_next_library = false;
-
   for (moduleIter = modules->begin(); moduleIter != modules->end(); ++moduleIter) {
     char moduleName[1024];
 
     (*moduleIter)->getName(moduleName, 1024);
-    if ((*moduleIter)->isSharedLib() && (strstr(moduleName, ".so.") != NULL || (strlen(moduleName) > 3 && strncmp(moduleName + strlen(moduleName) - 3, ".so", 3) == 0))) {
-      bool skip_this_lib = true;
-      for (std::set<std::string>::iterator libIter = instrumentLibraries.begin(); libIter != instrumentLibraries.end(); ++libIter)
-        if (strncmp(libIter->c_str(), moduleName, strlen(libIter->c_str())) == 0)
-          skip_this_lib = false;
-      if (skip_this_lib == true) {
-        skip_until_next_library = true;
+    if ((*moduleIter)->isSharedLib()) {
+      if (instrumentLibraries.find(moduleName) == instrumentLibraries.end() && string(moduleName).find(".so") != string::npos) {
         cout << "Skipping library: " << moduleName << endl;
         continue;
-      } else {
-        skip_until_next_library = false;
       }
     }
 
     if (string(moduleName).find(defaultModuleName) != string::npos) {
-      if (skipMainModule) {
-        skip_until_next_library = true;
+      if (skipMainModule)
         continue;
-      } else {
-        skip_until_next_library = false;
-      }
-    }
-
-    if (skip_until_next_library == true) {
-      if (verbose)
-        cout << "Skipping " << moduleName << " because skip_until_next_library is active" << endl;
-      continue;
     }
 
     if (do_bb == true) {
diff --git a/libAflDyninst.cpp b/libAflDyninst.cpp
index af2d9f4..c9ad158 100644
--- a/libAflDyninst.cpp
+++ b/libAflDyninst.cpp
@@ -1,4 +1,4 @@
-#include "config.h"
+#include "config.h" // do symlink: ln -s ../AFLplusplus/include afl
 #include "dyninstversion.h" // if this include errors, compile and install https://github.com/dyninst/dyninst
 #include <algorithm>
 #include <cstdio>
@@ -23,8 +23,8 @@ static pid_t __afl_fork_pid;
 static unsigned short int prev_id = 0;
 static bool forkserver_installed = false;
 #if (__amd64__ || __x86_64__)
-static long saved_di;
 #if (DYNINST_MAJOR_VERSION < 10)
+static long saved_di;
 register long rdi asm("di"); // the warning is fine - we need the warning because of a bug in dyninst9
 #endif
 #endif
@@ -100,9 +100,7 @@ void save_rdi() {
   saved_di = rdi;
 #endif
 }
-#endif
 
-#if (DYNINST_MAJOR_VERSION < 10)
 void restore_rdi() {
 #if __amd64__ || __x86_64__
   rdi = saved_di;