Don't crash if the PSBT is junk #5506
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustyrussell
approved these changes
Aug 9, 2022
rustyrussell
requested changes
Aug 9, 2022
We call `tal_wally_start` and then `tal_wally_start` again in `type_to_string` for psbt. end the last tal before calling type to string. Fixes: ElementsProject#5499 Reported-By: @fiatjaf lightning_hsmd: FATAL SIGNAL 6 (version v0.11.0.1-231-gddf8fbd-modded) 0x5574ca0d87ef send_backtrace common/daemon.c:33 0x5574ca0d8877 crashdump common/daemon.c:46 0x7f76ef63e8df ??? ???:0 0x7f76ef68e36c ??? ???:0 0x7f76ef63e837 ??? ???:0 0x7f76ef628534 ??? ???:0 0x5574ca0df55e tal_wally_start common/utils.c:27 0x5574ca0e4024 psbt_to_b64 bitcoin/psbt.c:687 0x5574ca0e4093 fmt_wally_psbt_ bitcoin/psbt.c:694 0x5574ca0df4b9 type_to_string_ common/type_to_string.c:32 0x5574ca0d5139 sign_our_inputs hsmd/libhsmd.c:486 0x5574ca0d5206 handle_sign_withdrawal_tx hsmd/libhsmd.c:1029 0x5574ca0d63c4 hsmd_handle_client_message hsmd/libhsmd.c:1575 0x5574ca0ce763 handle_client hsmd/hsmd.c:671 0x5574ca100032 next_plan ccan/ccan/io/io.c:59 0x5574ca1004b9 do_plan ccan/ccan/io/io.c:407 0x5574ca100552 io_ready ccan/ccan/io/io.c:417 0x5574ca101daf io_loop ccan/ccan/io/poll.c:453 0x5574ca0ceb7b main hsmd/hsmd.c:739 0x7f76ef62928f ??? ???:0 0x7f76ef629349 ??? ???:0 0x5574ca0cda04 ??? ../sysdeps/x86_64/start.S:115
If you build a PSBT externally from CLN and attempt to sign for the output, we would crash. Now we don't crash. Changelog-Changed: JSON-RPC: `signpsbt` will now add redeemscript + witness-utxo to the PSBT for an input that we can sign for, before signing it. Fixes ElementsProject#5499 ?
niftynei
force-pushed
the
nifty/psbt-boom
branch
from
14d681e
to
b7c3b29
Compare
Changelog-Changed: JSON-RPC: `signpsbt` no longer crashes if it doesn't like what your PSBT is
rustyrussell
approved these changes
Aug 13, 2022
|
Ack b7c3b29 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If you pass in a PSBT that's invalid (no inputs/outputs for example), lightningd blows up in the HSM.
Instead, let's catch bad PSBTs before we get that far.
Also uncovered a kinda fun "double entrance" into
tal_wallycode.Waiting for confirmation from @fiatjaf