fuzzing: Add ut_process application setup #18802
Conversation
(under the assumption that the first statement is correct 😛) |
There was a problem hiding this comment.
Very cool that someone else is using the fuzzing stuff as well 🤗 I primarily used the fuzzing/ applications to test GNRC network modules by injecting inputs over the IPC message passing mechanism. So this is a sightly different use-case but it is good idea to also use fuzzing for testing individual library functions.
is either means the ut_process has no bugs detectable by the AFL or my setup was incorrect.
You can also evaluate your setup by checking achieved coverage results. On native this should be rather straight forward. Furthermore, you also only have a single input in your input corpus which may make it difficult for AFL to generate "interesting" inputs which cover new parts of the module (see also: the afl-cmin documentation).
fuzzing/ut_process/main.c
Outdated
| char input_buf[BUFF_SIZE]; | ||
| char output_buf[BUFF_SIZE]; | ||
|
|
||
| ssize_t input_len = read(STDIN_FILENO, input_buf, BUFF_SIZE); |
There was a problem hiding this comment.
Would be more appropriate to read until EOF here. The fuzzing_read_packet packet function from the sys/fuzzing module does that already but returns a gnrc_pktsnip_t as an output parameter (as I mentioned earlier I primarily used fuzzing for testing GNRC network modules). Maybe that function could be generalized a bit and re-used for this purpose.
There was a problem hiding this comment.
Meaning I would dynamically increase the input_buf size and thereby removing the limit / constraint of BUFF_SIZE from AFL input generation?
fuzzing/ut_process/main.c
Outdated
| /* typically looks like this: "http://www.example.com/foo{?query,number}" */ | ||
| ut_process_var_t vars[] = { | ||
| { .name = input_buf, .value = &input_buf[11], }, | ||
| { .name = &input_buf[22], .value = &input_buf[33], }, | ||
| }; | ||
| size_t res_buf_len = BUFF_SIZE; | ||
|
|
||
| ut_process_str_expand(&input_buf[44], vars, 2, output_buf, &res_buf_len); |
There was a problem hiding this comment.
Isn't it somewhat annoying to create an input seed if you split the input like that? Wouldn't it make more sense to supply vars[] separately? Not sure though if there is a good way to have AFL generate/supply multiple inputs.
Also: How is null-termination of the value member enforced?
There was a problem hiding this comment.
Isn't it somewhat annoying [..]
Yes.
Wouldn't it make more sense to supply vars[] separately?
Yes.
Not sure though if there is a good way to have AFL generate/supply multiple inputs.
Me neither. I looked a bit through the documentation & best practices without success.
One approach could be using defining grammar as described here: lcamtuf.blogspot.com/2015/01/afl-fuzz-making-up-grammar-with.html. At the moment I'm not willig / able to go down that road :/
How is null-termination of the value member enforced?
It's not. And from my perspective its a feature, otherwise AFL could not test non-null terminated input.
(Also @nmeum in reply to that, GitHub external, translated from German)
the fuzzer will generate a lot of inputs where there is no null terminator for the variable names and then you have a lot of inputs that are rejected early and where no meaningful exploration of this
ut_expandfunction takes place (at least this is my assumption, which would also be seen in the coverage typically)
There was a problem hiding this comment.
It's not. And from my perspective its a feature, otherwise AFL could not test non-null terminated input.
In this case null-termination of the .value strings is part of the ut_process_str_expand API contract, isn't it? Also: since the entire input_buf is null-terminated .value will also always be null-terminated it is just that it sometimes uses the entirety of the input_buf for .value then.
fuzzing/ut_process/Makefile
Outdated
| # Comment this out to disable code in RIOT that does safety checking | ||
| # which is not needed in a production environment but helps in the | ||
| # development process: | ||
| DEVELHELP ?= 1 |
There was a problem hiding this comment.
DEVELHELP is enabled by default by Makefile.fuzzing_common.
bf08fbd to
1966572
Compare
|
@nmeum based on your feedback, I slightly changed the setup. I also experimented with persistent fuzzing but sadly the LLVM support in RIOT isn't good enough yet. |
|
Closed in favour of #19057 |
19057: fuzzing: Add uri_parser setup r=benpicco a=Teufelchen1 Hello! ### Contribution description This PR is a replacement for PR #18802 In this contribution: * The variable `AFL_FLAGS` is renamed to `FLAGS_FOR_AFL` because AFL is always complaining that `AFL_FLAGS` is not a valid env var for it. While this is not a bug nor an issue, I found it to be annoying. * A generic input reader is added to simplify building a test harness * The usage of this reader is demonstrated by adding a harness for fuzzing the uri_parser (needs squashing after review) ### Testing procedure Go to `fuzzing/uri_parser` and run `make all-asan` and `make fuzz` to get some action going. Also mildly interesting: `./dist/tools/compile_test/compile_like_murdock.py -b native -a fuzzing/uri_parser` ### Issues/PRs references The original PR #18802 is replaced because the generic input reader is present in both PRs but this PoC harness is much simpler. 19151: examples/gcoap: Fix shell parameter validation r=benpicco a=maribu ### Contribution description Executing the shell command with an URI-Path that doesn't start with a slash results in an assertion error while composing the client side message. This is suboptimal user experience, so add an explicit check for a valid URI-Path and a dedicated error message. ### Testing procedure #### In `master` ``` $ make BOARD=microbit-v2 -C examples/gcoap flash term [...] 2023-01-15 22:23:32,512 # coap get [::1] /.well-known/core 2023-01-15 22:23:32,516 # gcoap_cli: sending msg ID 52272, 23 bytes 2023-01-15 22:23:32,520 # gcoap: response Success, code 2.05, 46 bytes 2023-01-15 22:23:32,524 # </cli/stats>;ct=0;rt="count";obs,</riot/board> > coap get [::1] foo 2023-01-15 22:23:34,763 # coap get [::1] foo 2023-01-15 22:23:34,763 # 2329 2023-01-15 22:23:34,765 # *** RIOT kernel panic: 2023-01-15 22:23:34,767 # FAILED ASSERTION. 2023-01-15 22:23:34,767 # 2023-01-15 22:23:34,775 # pid | name | state Q | pri | stack ( used) ( free) | base addr | current 2023-01-15 22:23:34,784 # - | isr_stack | - - | - | 512 ( 200) ( 312) | 0x20000000 | 0x200001c8 2023-01-15 22:23:34,793 # 1 | main | running Q | 7 | 1536 ( 1072) ( 464) | 0x200006c0 | 0x2000095c 2023-01-15 22:23:34,802 # 2 | 6lo | bl rx _ | 3 | 1024 ( 328) ( 696) | 0x200036c0 | 0x200039c4 2023-01-15 22:23:34,810 # 3 | ipv6 | bl rx _ | 4 | 1024 ( 460) ( 564) | 0x20001294 | 0x20001574 2023-01-15 22:23:34,819 # 4 | udp | bl rx _ | 5 | 512 ( 300) ( 212) | 0x20003e98 | 0x20003f9c 2023-01-15 22:23:34,828 # 5 | coap | bl anyfl _ | 6 | 1112 ( 704) ( 408) | 0x20000e38 | 0x200011c4 2023-01-15 22:23:34,837 # 6 | nrf802154 | bl anyfl _ | 2 | 896 ( 288) ( 608) | 0x20001a90 | 0x20001d54 2023-01-15 22:23:34,843 # | SUM | | | 6616 ( 3352) ( 3264) 2023-01-15 22:23:34,843 # 2023-01-15 22:23:34,844 # *** halted. 2023-01-15 22:23:34,844 # ``` #### This PR ``` $ make BOARD=microbit-v2 -C examples/gcoap flash term [...] make: Entering directory '/home/maribu/Repos/software/RIOT/examples/gcoap' /home/maribu/Repos/software/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" 2023-01-15 22:22:27,842 # Connect to serial port /dev/ttyACM0 Welcome to pyterm! Type '/exit' to exit. coap get [::1] /.well-known/core 2023-01-15 22:22:40,042 # coap get [::1] /.well-known/core 2023-01-15 22:22:40,046 # gcoap_cli: sending msg ID 25182, 23 bytes 2023-01-15 22:22:40,050 # gcoap: response Success, code 2.05, 46 bytes 2023-01-15 22:22:40,054 # </cli/stats>;ct=0;rt="count";obs,</riot/board> > coap get [::1] foo 2023-01-15 22:22:43,858 # coap get [::1] foo 2023-01-15 22:22:43,862 # ERROR: URI-Path must start with a "/" 2023-01-15 22:22:43,866 # usage: coap <get|post|put|ping|proxy|info> ``` ### Issues/PRs references None Co-authored-by: Teufelchen1 <bennet.blischke@haw-hamburg.de> Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
Hi 👋
This PR adds a new fuzzing setup for the ut_process module.
I'm fairly new to fuzzing, so an in-depth review by someone experienced would be great! 👀 @nmeum
I've run this setup for ~84 hours, finished 825 cycles and 188 without finds. No crashes nor hangs were found resulting in a reported stability of 100%. This either means the ut_process has no bugs detectable by the AFL or my setup was incorrect.
For testing follow the regular fuzzing instructions.