Skip to content

channeld: initialize tx_sigs_allowed on startup - #9483

Open
erickcestari wants to merge 1 commit into
ElementsProject:masterfrom
erickcestari:fix-ubsan-tx-sigs-allowed
Open

channeld: initialize tx_sigs_allowed on startup#9483
erickcestari wants to merge 1 commit into
ElementsProject:masterfrom
erickcestari:fix-ubsan-tx-sigs-allowed

Conversation

@erickcestari

Copy link
Copy Markdown
Contributor

peer->tx_sigs_allowed was only set in peer_reconnect() and when we receive channel_ready, so on a fresh channeld start a stray tx_signatures arriving before the peer's channel_ready made handle_unexpected_tx_sigs() read an uninitialized bool. UBSan flags this as a load of an invalid bool value, and in normal builds the "warn and disconnect" branch was effectively taken at random.

Default it to false: we only allow an unexpected tx_signatures when reconnecting.

Changelog-None

I've found this issue using Smite.

The Impact is minimal: the flag only decides whether one unexpected tx_signatures is logged and dropped, or answered with a warning and a disconnect. The message is never acted on either way, so no channel state or funds are affected; the worst case is that a peer gets one stray message ignored instead of being disconnected.

peer->tx_sigs_allowed was only set in peer_reconnect() and when we
receive channel_ready, so on a fresh channeld start a stray
tx_signatures arriving before the peer's channel_ready made
handle_unexpected_tx_sigs() read an uninitialized bool.  UBSan flags
this as a load of an invalid bool value, and in normal builds the
"warn and disconnect" branch was effectively taken at random.

Default it to false: we only allow an unexpected tx_signatures when
reconnecting.

Changelog-None
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.

1 participant