fuzz-tests: Add a test for the gossipd-connectd interface #8423

Chand-ra · 2025-07-22T12:19:37Z

connectd_req() in gossipd/gossipd.c is responsible for handling gossip messages from peers handed to it by connectd. Add a stateful test simulating its behaviour.

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

Chand-ra · 2025-07-22T12:21:19Z

The test results in the following LeakSanitizer error when run on its corpus:

==116428==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x619eb51001f3 in malloc (/home/chandra/lightning/tests/fuzz/fuzz-gossipd-connectd+0x2da1f3) (BuildId: 34398486c14ff41d79a297932e621cb4f6161418)
    #1 0x619eb54deb3d in split_node /home/chandra/lightning/ccan/ccan/intmap/intmap.c:67:9
    #2 0x619eb54de8c2 in intmap_add_ /home/chandra/lightning/ccan/ccan/intmap/intmap.c:113:9
    #3 0x619eb53122d1 in map_add /home/chandra/lightning/gossipd/gossmap_manage.c:176:6
    #4 0x619eb530f8c6 in gossmap_manage_channel_announcement /home/chandra/lightning/gossipd/gossmap_manage.c:640:8
    #5 0x619eb53b2101 in handle_recv_gossip /home/chandra/lightning/tests/fuzz/../../gossipd/gossipd.c:205:12
    #6 0x619eb53a93b2 in run /home/chandra/lightning/tests/fuzz/fuzz-gossipd-connectd.c:543:4
    #7 0x619eb513e8f8 in LLVMFuzzerTestOneInput /home/chandra/lightning/tests/fuzz/libfuzz.c:25:2
    #8 0x619eb504c0c4 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/chandra/lightning/tests/fuzz/fuzz-gossipd-connectd+0x2260c4) (BuildId: 34398486c14ff41d79a297932e621cb4f6161418)
    #9 0x619eb504b7b9 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/home/chandra/lightning/tests/fuzz/fuzz-gossipd-connectd+0x2257b9) (BuildId: 34398486c14ff41d79a297932e621cb4f6161418)
    #10 0x619eb504d3d6 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/chandra/lightning/tests/fuzz/fuzz-gossipd-connectd+0x2273d6) (BuildId: 34398486c14ff41d79a297932e621cb4f6161418)
    #11 0x619eb504d8e7 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/chandra/lightning/tests/fuzz/fuzz-gossipd-connectd+0x2278e7) (BuildId: 34398486c14ff41d79a297932e621cb4f6161418)
    #12 0x619eb503addf in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/chandra/lightning/tests/fuzz/fuzz-gossipd-connectd+0x214ddf) (BuildId: 34398486c14ff41d79a297932e621cb4f6161418)
    #13 0x619eb5065466 in main (/home/chandra/lightning/tests/fuzz/fuzz-gossipd-connectd+0x23f466) (BuildId: 34398486c14ff41d79a297932e621cb4f6161418)
    #14 0x7aed7c22a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #15 0x7aed7c22a28a in __libc_start_main csu/../csu/libc-start.c:360:3
    #16 0x619eb502fdc4 in _start (/home/chandra/lightning/tests/fuzz/fuzz-gossipd-connectd+0x209dc4) (BuildId: 34398486c14ff41d79a297932e621cb4f6161418)

SUMMARY: AddressSanitizer: 40 byte(s) leaked in 1 allocation(s).

INFO: a leak has been found in the initial corpus.

morehouse

High level review: I like the target. Needs some rework so we consider exit to be a crash.

Haven't studied create_gossip_msg yet.

morehouse · 2025-07-28T18:06:03Z

tests/fuzz/fuzz-gossipd-connectd.c

+			struct node_id id = node_id(privkey_from_index(tal_count(peer_ids)));
+			tal_arr_expand(&peer_ids, id);
+
+			msg = towire_gossipd_new_peer(tmpctx, &id, false);


Setting gossip_queries_feature = false will limit some of the code paths that can be executed, especially all the seeker stuff. If there's a good reason to do this, please add a comment here that explains why.

morehouse · 2025-07-28T18:24:46Z

tests/fuzz/fuzz-gossipd-connectd.c

+			switch (fromwire_u8(&data, &size) % 5)
+			{
+				case 0:
+					gossip_msg = create_gossip_msg(tmpctx, &data, &size, WIRE_CHANNEL_ANNOUNCEMENT);
+					break;
+				case 1:
+					gossip_msg = create_gossip_msg(tmpctx, &data, &size, WIRE_CHANNEL_UPDATE);
+					break;
+				case 2:
+					gossip_msg = create_gossip_msg(tmpctx, &data, &size, WIRE_NODE_ANNOUNCEMENT);
+					break;
+				case 3:
+					gossip_msg = create_gossip_msg(tmpctx, &data, &size, WIRE_REPLY_CHANNEL_RANGE);
+					break;
+				case 4:
+					gossip_msg = create_gossip_msg(tmpctx, &data, &size, WIRE_REPLY_SHORT_CHANNEL_IDS_END);
+					break;
+				default:
+					break;
+			}


Nit: perhaps this switch could go inside create_gossip_msg, so there's one less parameter to pass.

morehouse · 2025-07-28T19:41:41Z

tests/fuzz/fuzz-gossipd-connectd.c

+
+cleanup:
+	if (daemon)
+		tal_free(daemon->connectd);


Is the explicit tal_free necessary? Naively it looks like clean_tmpctx should take care of this already.

This is to circumvent the dangling allocation error fixed in #8424 .

morehouse · 2025-07-28T19:43:36Z

tests/fuzz/fuzz-gossipd-connectd.c

+	if (setjmp(exit_jmp) != 0)
+		goto cleanup;


For gossipd, we actually shouldn't consider exit to be normal. If gossipd exits, the entire CLN node shuts down, which would be a DoS vulnerability.

This path can only be triggered by status_failed() and towire_warningfmt().

I can get rid of the mock for towire_warningfmt() by including common/wire_error.o for linking. As for status_failed(), I don't think triggering it would mean a vulnerability, it's used pretty liberally throughout gossipd/gossipd.c to report a bad message from the peer. Maybe exit()'s behavior manifests differently in a live node than it does in our fuzzer (where it aborts the entire process).

I don't see any reason for us to quit the fuzz target for towire_warning_fmt.

status_failed means "print error and exit", and should never happen in the wild. If it does, it's a DoS vulnerability.

morehouse · 2025-07-28T21:34:39Z

The test results in the following LeakSanitizer error when run on its corpus:

I looked closer at this. The accused line of code does a naked malloc (no tal use), which makes me suspect there could be a cleanup problem on each iteration of the fuzz target, since only tal-allocated memory gets freed.

We might need to manually delete all elements from the gossmap at the end of each iteration.

Chand-ra · 2025-07-29T10:06:47Z

We might need to manually delete all elements from the gossmap at the end of each iteration.

This makes the target a bit messier but does seem to manage to evade the issue at hand.

Changelon-None: `connectd_req()` in `gossipd/gossipd.c` is responsible for handling gossip messages from peers handed to it by `connectd`. Add a stateful test simulating its behaviour.

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 mentioned this pull request Jul 22, 2025

common: fix dangling memory allocation in daemon_conn_new_() #8424

Open

4 tasks

morehouse suggested changes Jul 28, 2025

View reviewed changes

Chandra Pratap added 2 commits July 29, 2025 10:11

fuzz-tests: Add a test for the gossipd-connectd interface

68d0cbc

Changelon-None: `connectd_req()` in `gossipd/gossipd.c` is responsible for handling gossip messages from peers handed to it by `connectd`. Add a stateful test simulating its behaviour.

fuzz-tests: Add a seed corpus for the new test

4721eb2

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 force-pushed the gossipd-connectd branch from 448e689 to 4721eb2 Compare July 29, 2025 10:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fuzz-tests: Add a test for the gossipd-connectd interface #8423

fuzz-tests: Add a test for the gossipd-connectd interface #8423

Chand-ra commented Jul 22, 2025

Uh oh!

Chand-ra commented Jul 22, 2025

Uh oh!

morehouse left a comment

Uh oh!

morehouse Jul 28, 2025

Uh oh!

morehouse Jul 28, 2025

Uh oh!

morehouse Jul 28, 2025

Uh oh!

Chand-ra Jul 29, 2025

Uh oh!

morehouse Jul 28, 2025

Uh oh!

Chand-ra Jul 29, 2025

Uh oh!

morehouse Jul 29, 2025

Uh oh!

morehouse commented Jul 28, 2025

Uh oh!

Chand-ra commented Jul 29, 2025

Uh oh!

Uh oh!

fuzz-tests: Add a test for the gossipd-connectd interface #8423

Are you sure you want to change the base?

fuzz-tests: Add a test for the gossipd-connectd interface #8423

Conversation

Chand-ra commented Jul 22, 2025

Checklist

Uh oh!

Chand-ra commented Jul 22, 2025

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

Uh oh!

morehouse Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

morehouse Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

morehouse Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Chand-ra Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

morehouse Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Chand-ra Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

morehouse Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

morehouse commented Jul 28, 2025

Uh oh!

Chand-ra commented Jul 29, 2025

Uh oh!

Uh oh!