diff --git a/channeld/commit_tx.c b/channeld/commit_tx.c index 1ee72406f..ca2d232ce 100644 --- a/channeld/commit_tx.c +++ b/channeld/commit_tx.c @@ -242,6 +242,14 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx, n++; } + /* BOLT #2: + * + * - MUST set `channel_reserve_satoshis` greater than or equal to + * `dust_limit_satoshis`. + */ + /* This means there must be at least one output. */ + assert(n > 0); + assert(n <= tx->wtx->outputs_allocation_len); tal_resize(htlcmap, n); diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c index ca9380345..77f3897f0 100644 --- a/hsmd/hsmd.c +++ b/hsmd/hsmd.c @@ -152,6 +152,9 @@ static PRINTF_FMT(4,5) master_badmsg(fromwire_peektype(msg_in), msg_in); } + /*~ Nobody should give us bad requests; it's a sign something is broken */ + status_broken("%s: %s", type_to_string(tmpctx, struct node_id, &c->id), str); + /*~ Note the use of NULL as the ctx arg to towire_hsmstatus_: only * use NULL as the allocation when we're about to immediately free it * or hand it off with take(), as here. That makes it clear we don't @@ -770,6 +773,12 @@ static struct io_plan *handle_sign_commitment_tx(struct io_conn *conn, &funding)) return bad_req(conn, c, msg_in); + /* Basic sanity checks. */ + if (tx->wtx->num_inputs != 1) + return bad_req_fmt(conn, c, msg_in, "tx must have 1 input"); + if (tx->wtx->num_outputs == 0) + return bad_req_fmt(conn, c, msg_in, "tx must have > 0 outputs"); + get_channel_seed(&peer_id, dbid, &channel_seed); derive_basepoints(&channel_seed, &local_funding_pubkey, NULL, &secrets, NULL); @@ -823,6 +832,12 @@ static struct io_plan *handle_sign_remote_commitment_tx(struct io_conn *conn, &funding)) bad_req(conn, c, msg_in); + /* Basic sanity checks. */ + if (tx->wtx->num_inputs != 1) + return bad_req_fmt(conn, c, msg_in, "tx must have 1 input"); + if (tx->wtx->num_outputs == 0) + return bad_req_fmt(conn, c, msg_in, "tx must have > 0 outputs"); + get_channel_seed(&c->id, c->dbid, &channel_seed); derive_basepoints(&channel_seed, &local_funding_pubkey, NULL, &secrets, NULL); diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index b641feb3c..cc0362871 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -1224,6 +1224,21 @@ static bool changed_htlc(struct channel *channel, return update_in_htlc(channel, changed->id, changed->newstate); } +/* FIXME: This should be a complete check, not just a sanity check. + * Perhaps that means we need a cookie from the HSM? */ +static bool valid_commitment_tx(struct channel *channel, + const struct bitcoin_tx *tx) +{ + /* We've had past issues where all outputs are trimmed. */ + if (tx->wtx->num_outputs == 0) { + channel_internal_error(channel, + "channel_got_commitsig: zero output tx! %s", + type_to_string(tmpctx, struct bitcoin_tx, tx)); + return false; + } + return true; +} + static bool peer_save_commitsig_received(struct channel *channel, u64 commitnum, struct bitcoin_tx *tx, const struct bitcoin_signature *commit_sig) @@ -1236,6 +1251,10 @@ static bool peer_save_commitsig_received(struct channel *channel, u64 commitnum, return false; } + /* Basic sanity check */ + if (!valid_commitment_tx(channel, tx)) + return false; + channel->next_index[LOCAL]++; /* Update channel->last_sig and channel->last_tx before saving to db */