-
Notifications
You must be signed in to change notification settings - Fork 942
fuzz-tests: Add a test for peer_init_received()
#8385
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
base: master
Are you sure you want to change the base?
Conversation
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.
This test also triggers the bug that is fixed in #8325. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fuzz target leaks memory because peer_init_received
uses tmpctx
and clean_tmpctx
is never called.
Also we should add better comments, given the complexity of this target.
@@ -0,0 +1,161 @@ | |||
#include "config.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intercepting io_read
and io_write
is tricky, so please document exactly why we do this and what we are trying to accomplish here.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably just use tmpctx
for simplicity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
peer_init_received()
inconnectd/peer_exchange_initmsg.{c, h}
is responsible for handlinginit
messages 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