Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -4417,6 +4417,12 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg)

type = fromwire_peektype(inmsg);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't the check placed too late to do anything? Look at the top of the function -

static void splice_initiator(struct peer *peer, const u8 *inmsg)
{
	/ some code /
	struct wally_psbt *psbt = peer->splicing->current_psbt;   - on line 4412
	/ some code /
	type = fromwire_peektype(inmsg);  - on line 4418

	if (!peer->splicing) {  - and here we have new  check
		peer_failed_warn(...);
	}
}

So psbt is initialized from peer->splicing->current_psbt in the declaration block, which executes before type = fromwire_peektype(inmsg) and before the new if (!peer->splicing) check. If peer->splicing is NULL, the function already dereferences it at line 4412 and crashes - so the new check is unreachable code?

Maybe we should move the NULL check to the very first line of the function, before psbt is declared/initialized?

if (!peer->splicing) {
peer_failed_warn(peer->pps, &peer->channel_id,
"Received %s without an active splice session",
peer_wire_name(type));
}

if (type == WIRE_SPLICE_ACK) {
struct tlv_splice_ack_tlvs *splice_ack_tlvs;
if (!fromwire_splice_ack(tmpctx, inmsg,
Expand Down
Loading