common: remove take() leak if new_initial_channel() fails.
What the developer wrote
Authored by Rusty Russell
common: remove take() leak if new_initial_channel() fails.
This happens in the fuzzer corpora, but that doesn't check for take()
leaks. Our unit tests do:
```
fuzz-initial_channel: outstanding taken(): 0x626c3b3affc8
make: *** [Makefile:823: unittest/tests/fuzz/fuzz-initial_channel] Error 1
```
This doesn't matter in real life, since we exit the subdaemon if this
fails, but it's still a bug.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This commit fixes a small memory leak in a helper function used when setting up a new Lightning channel. The leak only occurs if the function fails partway through, and the project already exits the affected sub-program when that happens, so it is not considered a real-world security issue. The fix moves a few memory allocations to the very beginning of the function so they can be cleaned up correctly if an error occurs later.
No urgent action needed. Treat as routine code-quality / defensive fix. If running fuzzers or unit tests, ensure the updated test passes. No security advisory or patch deployment is required on its own.
Security signals we found
Memory leak / resource leak in error path
Detected by unit-test take() leak checker
Error path in channel initialization helper
Evidence from the diff
In common/initial_channel.c, new_initial_channel() now duplicates fee_states, height_states, and channel->type before populating the rest of the struct. Previously these allocations happened after several assignments that could fail, so the take()/tal memory tracker would report outstanding allocations if the function returned early. The commit message explicitly states this is a fuzzer/unit-test hygiene issue and does not matter in production because the subdaemon exits on failure.
Changed components
common/initial_channel.cnew_initial_channel()Inspect captured patch +13 / −12
diff --git a/common/initial_channel.c b/common/initial_channel.c
index bbda5c58..417b766c 100644
--- a/common/initial_channel.c
+++ b/common/initial_channel.c
@@ -31,6 +31,19 @@ struct channel *new_initial_channel(const tal_t *ctx,
struct channel *channel = tal(ctx, struct channel);
struct amount_msat remote_msatoshi;
+ /* takes() if necessary */
+ channel->fee_states = dup_fee_states(channel, fee_states);
+
+ /* takes() if necessary */
+ if (!height_states)
+ channel->blockheight_states = NULL;
+ else
+ channel->blockheight_states
+ = dup_height_states(channel, height_states);
+
+ /* takes() if necessary */
+ channel->type = tal_dup(channel, struct channel_type, type);
+
channel->cid = *cid;
channel->funding = *funding;
channel->funding_sats = funding_sats;
@@ -47,16 +60,6 @@ struct channel *new_initial_channel(const tal_t *ctx,
channel->funding_pubkey[REMOTE] = *remote_funding_pubkey;
channel->htlcs = NULL;
- /* takes() if necessary */
- channel->fee_states = dup_fee_states(channel, fee_states);
-
- /* takes() if necessary */
- if (!height_states)
- channel->blockheight_states = NULL;
- else
- channel->blockheight_states
- = dup_height_states(channel, height_states);
-
channel->view[LOCAL].owed[LOCAL]
= channel->view[REMOTE].owed[LOCAL]
= local_msatoshi;
@@ -77,8 +80,6 @@ struct channel *new_initial_channel(const tal_t *ctx,
&channel->basepoints[!opener].payment);
channel->option_wumbo = option_wumbo;
- /* takes() if necessary */
- channel->type = tal_dup(channel, struct channel_type, type);
return channel;
}
Why this scored 20/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.