Feebdack - Githubissues

vanhauser-thc commented 3 years ago

I used the mruby example and got this just when starting up:

┘mutation error: No such file or directory

[-] PROGRAM ABORT : Error in custom_fuzz. Size returned: 0
         Location : fuzz_one_original(), src/afl-fuzz-one.c:1747

It should all be there:

AFL_CUSTOM_MUTATOR_ONLY=1
AFL_CUSTOM_MUTATOR_LIBRARY=/prg/Grammar-Mutator/trunk/src/libgrammarmutator.so
afl-fuzz -i in -o out -- mruby/bin/mruby @@
ls out/trees/
...
id:000070,time:0,orig:70  id:000156,time:0,orig:156  id:000242,time:0,orig:242
id:000071,time:0,orig:71  id:000157,time:0,orig:157  id:000243,time:0,orig:243
id:000072,time:0,orig:72  id:000158,time:0,orig:158  id:000244,time:0,orig:244
id:000073,time:0,orig:73  id:000159,time:0,orig:159  id:000245,time:0,orig:245
...

more feedback:

IMHO the GRAMMAR_FILE env var should always be required. having a JSON default is not helpful.
./grammar_generator 123 100 1000 /tmp/seeds /tmp/trees -> not found. it is src/grammar_generator. better copy the grammar_generator and the .so to the project root when done compiling, maybe even with the grammar type in their filename?

export export AFL_CUSTOM_MUTATOR_LIBRARY=/path/to/libgrammarmutator.so -> double export, also again below

dont put -o to /tmp, this is not best practice. just leave paths away so the example work in the the current directory

vanhauser-thc commented 3 years ago

More: if there is no trees/ subdirecty I dont see one being created and trees saved. Is this on purpose? is the trees/ directory only for startup? When I start without a trees directory I get the same error as above.

Also please update the README if/how you can use seeds that do not come from the ./grammar_generator. It is unclear from reading if this is possible or not.

Sadly I cannot test more as this error prevents me testing. :-( @h1994st @andreafioraldi - any workaround? what do I do wrong?

vanhauser-thc commented 3 years ago

the bug happens in rules_mutation as the node pointer is NULL

vanhauser-thc commented 3 years ago

this code fragment looks pointless:

  size_t      fn_len = strlen(fn);
  strncpy(data->new_tree_fn, fn, fn_len);
  data->new_tree_fn[fn_len] = '\0';

this is basically a strcpy(data->new_tree_fn, fn) just split to be 3 lines instead of one. if you dont want a buffer overflow (unlikely with PATH_MAX though) switch fn_len with PATH_MAX - 1 in line 2 and 3

vanhauser-thc commented 3 years ago

final comment before I have to leave for the weekend - please set -Wall -Wextra and make it compile cleanly before the final submit on Monday.

I will be back on Sunday afternoon and continue testing. I will have no laptop until then and very bad Internet ...

andreafioraldi commented 3 years ago

I'll test it again today. @h1994st when you will need this repo public? Before 31?

h1994st commented 3 years ago

More: if there is no trees/ subdirecty I dont see one being created and trees saved. Is this on purpose? is the trees/ directory only for startup?

When I start without a trees directory I get the same error as above.

Also please update the README if/how you can use seeds that do not come from the ./grammar_generator. It is unclear from reading if this is possible or not.

Sadly I cannot test more as this error prevents me testing. :-(

@h1994st @andreafioraldi - any workaround? what do I do wrong?

Thanks for your feedback!

Trees will be saved in out/trees. If it does not exist, it should be created. Let me double check it and the error.

I will try to address your comments today. Thanks! Enjoy your weekend.

h1994st commented 3 years ago

I'll test it again today. @h1994st when you will need this repo public? Before 31?

Thanks!

Maybe by the end of Aug 30 (Sunday)? So that I will have more time to prepare the submission.

h1994st commented 3 years ago

I used the mruby example and got this just when starting up:

┘mutation error: No such file or directory

[-] PROGRAM ABORT : Error in custom_fuzz. Size returned: 0
         Location : fuzz_one_original(), src/afl-fuzz-one.c:1747

It should all be there:

AFL_CUSTOM_MUTATOR_ONLY=1
AFL_CUSTOM_MUTATOR_LIBRARY=/prg/Grammar-Mutator/trunk/src/libgrammarmutator.so
afl-fuzz -i in -o out -- mruby/bin/mruby @@
ls out/trees/
...
id:000070,time:0,orig:70  id:000156,time:0,orig:156  id:000242,time:0,orig:242
id:000071,time:0,orig:71  id:000157,time:0,orig:157  id:000243,time:0,orig:243
id:000072,time:0,orig:72  id:000158,time:0,orig:158  id:000244,time:0,orig:244
id:000073,time:0,orig:73  id:000159,time:0,orig:159  id:000245,time:0,orig:245
...

@vanhauser-thc

A quick question. Which version of AFL++ do you use?

Your error position and error messages match with afl-fuzz-one.c#L1747 in the stable branch of AFL++.

Since the grammar mutator requires afl_custom_fuzz_count, the latest AFL++ in the dev branch should be used.

I should mention the version requirement for AFL++ in README.md.

h1994st commented 3 years ago

this code fragment looks pointless:
  size_t      fn_len = strlen(fn);
  strncpy(data->new_tree_fn, fn, fn_len);
  data->new_tree_fn[fn_len] = '\0';
this is basically a strcpy(data->new_tree_fn, fn) just split to be 3 lines instead of one. if you dont want a buffer overflow (unlikely with PATH_MAX though) switch fn_len with PATH_MAX - 1 in line 2 and 3

It's my fault. I wrongly think strncpy is a safer version of strcpy, but the destination string after strncpy is sometimes not null-terminated. I changed it to snprintf, which also avoids the buffer overflow.

vanhauser-thc commented 3 years ago

All compiles and works now. however to make it compile I had to remove -Werror, as I got warnings for the strncpy calls:

grammar_mutator.c: In function ‘afl_custom_queue_get’:
grammar_mutator.c:109:5: error: ‘strncpy’ output truncated before terminating nul copying 5 bytes from a string of the same length [-Werror=stringop-truncation]
  109 |     strncpy(found + 1, "trees", 5);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
grammar_mutator.c:130:3: error: ‘strncpy’ output truncated before terminating nul copying 5 bytes from a string of the same length [-Werror=stringop-truncation]
  130 |   strncpy(found + 1, "trees", 5);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
grammar_mutator.c: In function ‘afl_custom_queue_new_entry’:
grammar_mutator.c:459:3: error: ‘strncpy’ output truncated before terminating nul copying 5 bytes from a string of the same length [-Werror=stringop-truncation]
  459 |   strncpy(found + 1, "trees", 5);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

h1994st commented 3 years ago

I will take a look, as CI and my local build did not throw such warnings.

h1994st commented 3 years ago

All compiles and works now. however to make it compile I had to remove -Werror, as I got warnings for the strncpy calls:

grammar_mutator.c: In function ‘afl_custom_queue_get’:
grammar_mutator.c:109:5: error: ‘strncpy’ output truncated before terminating nul copying 5 bytes from a string of the same length [-Werror=stringop-truncation]
  109 |     strncpy(found + 1, "trees", 5);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
grammar_mutator.c:130:3: error: ‘strncpy’ output truncated before terminating nul copying 5 bytes from a string of the same length [-Werror=stringop-truncation]
  130 |   strncpy(found + 1, "trees", 5);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
grammar_mutator.c: In function ‘afl_custom_queue_new_entry’:
grammar_mutator.c:459:3: error: ‘strncpy’ output truncated before terminating nul copying 5 bytes from a string of the same length [-Werror=stringop-truncation]
  459 |   strncpy(found + 1, "trees", 5);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

This error/warning (stringop-truncation) is newly introduced in gcc-8. I tried gcc-8 (8.4.0) on my machine, but no error occurred. What is the version of your compiler?

Although I could not reproduce the error, it should be fixed by replacing strncpy with memcpy.

@vanhauser-thc you can try the latest commit to see whether the error has been removed. I cannot test it on my own.

h1994st commented 3 years ago

All errors/comments in this issue post are resolved:

Use the latest afl-fuzz in dev branch: mitigate errors thrown by afl-fuzz (Location : fuzz_one_original(), src/afl-fuzz-one.c:1747)
d2e0131ecfc2d15b8c3a610a20482d3ab86d92bb: mark GRAMMAR_FILE env as required
658cae28ab1ddaec30fd2a679e5831bafc25eb27, https://github.com/AFLplusplus/Grammar-Mutator/commit/bd108a54d2276e64c56068075015e01dbb3b2430, and https://github.com/AFLplusplus/Grammar-Mutator/commit/6edec891585262642d662dde2d51b169b8da0d1b: create two symbolic links for libgrammarmutator.so and grammar_generator
e3b368e5b4c4a8c3f2d74acbfd34a7e4abcf7d89: fix typo (double export) in README.md
e3b368e5b4c4a8c3f2d74acbfd34a7e4abcf7d89: update README.md to show how to use existing fuzzing seeds instead of using generated seeds
ccb748cbc47133806422d1d2aa8fb6d74aa0682d: fix potential buffer overflow while copying the file path
d2e0131ecfc2d15b8c3a610a20482d3ab86d92bb and 20098f3184f9ee4229114142c5da48fba324e1ed: set -Wall -Wextra -Werror
6edec891585262642d662dde2d51b169b8da0d1b: avoid stringop-truncation errors

vanhauser-thc commented 3 years ago

I use gcc 10.1 :)

vanhauser-thc commented 3 years ago

yes it compiles clean now

vanhauser-thc commented 3 years ago

llvm 12 complained about a thing, fixed it. (globals are always initialized with zeroes, so need to initialize them anyway)

vanhauser-thc commented 3 years ago

@h1994st otherwise everything looks fine. please ensure that everything that is required for GSOC is there :)

h1994st commented 3 years ago

Thanks! I have submitted the final evaluation.

AFLplusplus / Grammar-Mutator

Feebdack #9