fuzz-tests: Add a test for peer_init_received()
#8385
Conversation
|
This test also triggers the bug that is fixed in #8325. |
| @@ -0,0 +1,161 @@ | |||
| #include "config.h" | |||
There was a problem hiding this comment.
Please add a header comment here explaining how this fuzz target works.
| static size_t io_buf_pos = 0; | ||
|
|
||
| static struct io_plan * | ||
| test_write(struct io_conn *conn, const void *data, size_t len, |
There was a problem hiding this comment.
Intercepting io_read and io_write is tricky, so please document exactly why we do this and what we are trying to accomplish here.
There was a problem hiding this comment.
Still missing documentation on what exactly test_read and test_write are doing, and why. If I can't figure out what's going on here, probably no one else can either.
tests/fuzz/fuzz-init_received.c
Outdated
| conn = io_new_conn(run_ctx, -1, do_nothing, NULL); | ||
| peer_init_received(conn, peer); | ||
|
|
||
| tal_free(run_ctx); |
There was a problem hiding this comment.
We should probably just use tmpctx for simplicity.
There was a problem hiding this comment.
Right, that's what I was doing initially but calling clean_tmpctx() within the target causes the following crash:
fuzz-init_received: ccan/ccan/tal/tal.c:428: void del_tree(struct tal_hdr *, const tal_t *, int): Assertion `!taken(from_tal_hdr(t))' failed.
==10716== ERROR: libFuzzer: deadly signal
#0 0x599305aa8e05 in __sanitizer_print_stack_trace (/home/chandra/lightning/tests/fuzz/fuzz-init_received+0x292e05) (BuildId: 2e1951a325c952a9be03b7b50fae5fe68183b738)
#1 0x599305a0291c in fuzzer::PrintStackTrace() (/home/chandra/lightning/tests/fuzz/fuzz-init_received+0x1ec91c) (BuildId: 2e1951a325c952a9be03b7b50fae5fe68183b738)
#2 0x5993059e89a7 in fuzzer::Fuzzer::CrashCallback() (/home/chandra/lightning/tests/fuzz/fuzz-init_received+0x1d29a7) (BuildId: 2e1951a325c952a9be03b7b50fae5fe68183b738)
#3 0x7a3aa644532f (/usr/lib/x86_64-linux-gnu/libc.so.6+0x4532f) (BuildId: 42c84c92e6f98126b3e2230ebfdead22c235b667)
#4 0x7a3aa649eb2b in __pthread_kill_implementation nptl/pthread_kill.c:43:17
#5 0x7a3aa649eb2b in __pthread_kill_internal nptl/pthread_kill.c:78:10
#6 0x7a3aa649eb2b in pthread_kill nptl/pthread_kill.c:89:10
#7 0x7a3aa644527d in raise signal/../sysdeps/posix/raise.c:26:13
#8 0x7a3aa64288fe in abort stdlib/abort.c:79:7
#9 0x7a3aa642881a in __assert_fail_base assert/assert.c:96:3
#10 0x7a3aa643b516 in __assert_fail assert/assert.c:105:3
#11 0x599305e202f5 in del_tree /home/chandra/lightning/ccan/ccan/tal/tal.c:428:2
#12 0x599305e1f93c in tal_free /home/chandra/lightning/ccan/ccan/tal/tal.c:532:3
#13 0x599305b92370 in clean_tmpctx /home/chandra/lightning/common/utils.c:112:3
#14 0x599305c9dad2 in run /home/chandra/lightning/tests/fuzz/fuzz-init_received.c:161:2
There was a problem hiding this comment.
Error seems to indicate that a pointer's ownership was previously "taken" by some other function. You'll need to figure out where that's happening to debug further.
There was a problem hiding this comment.
The error was apparently in the test's mock for peer_connected(). The function's actual definition "TAKES" the passed their_features parameter, and replicating that behavior in the mock seems to fix the issue.
| static size_t io_buf_pos = 0; | ||
|
|
||
| static struct io_plan * | ||
| test_write(struct io_conn *conn, const void *data, size_t len, |
There was a problem hiding this comment.
Still missing documentation on what exactly test_read and test_write are doing, and why. If I can't figure out what's going on here, probably no one else can either.
|
After some inspection, I noticed that On the other hand, Given this, I believe it's acceptable to replace both |
Changelog-None: `peer_init_received()` in `connectd/peer_exchange_initmsg.{c, h}`
is responsible for handling `init` messages defined in BOLT ElementsProject#1. Since it deals
with untrusted input, add a test for it.
Add a minimal input set as a seed corpus for the newly introduced test. This leads to discovery of interesting code paths faster.
peer_init_received()inconnectd/peer_exchange_initmsg.{c, h}is responsible for handlinginitmessages defined inBOLT #1. Since it deals with untrusted input, add a test for it.Checklist
Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:
CC: @morehouse