Skip to content

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Chand-ra
Copy link

peer_init_received() in connectd/peer_exchange_initmsg.{c, h} is responsible for handling init messages defined in BOLT #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:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

CC: @morehouse

Chandra Pratap added 2 commits June 30, 2025 03:48
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.
@Chand-ra
Copy link
Author

This test also triggers the bug that is fixed in #8325.

Copy link
Contributor

@morehouse morehouse left a 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"
Copy link
Contributor

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,
Copy link
Contributor

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);
Copy link
Contributor

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.

Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants