Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

channeld aborts on large `update_add_htlc` amount

Abierto
#9,576 1 comentario 1 reacción 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

@bhuvan-somisetty ya está trabajando en esto.

Desde el 8/10/2026.

  • #9610 de @bhuvan-somisetty — abierto

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
78/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
c
Área
networking

Línea de trabajo

Start in channeld/full_channel.c at add_htlc and inspect the balance check and existing amount validation. Add or update coverage in channeld/test/run-full_channel.c, then run the full_channel test. Done means oversized amount_msat values are rejected and no assertion or channeld abort occurs.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

An update_add_htlc whose amount_msat has bit 63 set (i.e. far above the total Bitcoin supply) is accepted by channeld because the balance check underflows. When channeld then builds the local commitment transaction to verify the peer's commitment_signed, an assert fires and channeld aborts.

Spec violation

Two spec requirements are broken:

  1. The amount is not a legal BOLT 1 amount:
When used to encode amounts, the previous fields MUST comply with the upper
bound of 21 million BTC:

* satoshi amounts MUST be at most `0x000775f05a074000`
* milli-satoshi amounts MUST be at most `0x1d24b2dfac520000`
  1. CLN crashes rather than failing gracefully as specified in BOLT 2:
A receiving node:
  - receiving an `amount_msat` that the sending node cannot afford ...:
    - SHOULD send a `warning` and close the connection, or send an `error` and fail the channel.

Root cause

There's no ceiling on the received amount_msat, so the value can have the top bit set. The balance check happens here:

https://github.com/ElementsProject/lightning/blob/de88443cded7197f56c4f9a999d07c4b9400c0d6/channeld/full_channel.c#L35-L42

with balance->msat being represented as a signed integer:

https://github.com/ElementsProject/lightning/blob/de88443cded7197f56c4f9a999d07c4b9400c0d6/channeld/full_channel.c#L24-L26

Because the top bit of the HTLC amount is set, the subtraction underflows and actually increases balance->msat.

The subsequent balance_ok then passes because balance->msat is positive:

https://github.com/ElementsProject/lightning/blob/de88443cded7197f56c4f9a999d07c4b9400c0d6/channeld/full_channel.c#L65-L72

The HTLC is accepted.

Later, when the commitment transaction is built, one of two assertions is triggered:

  1. https://github.com/ElementsProject/lightning/blob/de88443cded7197f56c4f9a999d07c4b9400c0d6/channeld/commit_tx.c#L153
  2. https://github.com/ElementsProject/lightning/blob/de88443cded7197f56c4f9a999d07c4b9400c0d6/bitcoin/tx.c#L76

Impact

channeld crashes rather than failing gracefully. No other daemons are affected.

Reproduction

diff --git a/channeld/test/run-full_channel.c b/channeld/test/run-full_channel.c
--- a/channeld/test/run-full_channel.c
+++ b/channeld/test/run-full_channel.c
@@ -205,6 +205,31 @@ static const struct htlc **include_htlcs(struct channel *channel, enum side side
 	return htlcs;
 }
 
+/* A received update_add_htlc over the BOLT #1 amount ceiling must be refused,
+ * not admitted via signed-overflow of channeld's balance check. */
+static void test_reject_oversized_htlc(struct channel *channel)
+{
+	const u64 max_msat = 0x1d24b2dfac520000ULL; /* BOLT #1: 21M BTC */
+	u8 *dummy_routing = tal_arr(tmpctx, u8,
+				    TOTAL_PACKET_SIZE(ROUTING_INFO_SIZE));
+	struct sha256 hash;
+	enum channel_add_err e;
+
+	memset(&hash, 0, sizeof(hash));
+
+	e = channel_add_htlc(channel, REMOTE, 0, AMOUNT_MSAT(UINT64_MAX),
+			     500, &hash, dummy_routing,
+			     NULL, NULL, NULL, NULL, false);
+	assert(e != CHANNEL_ERR_ADD_OK);
+
+	e = channel_add_htlc(channel, REMOTE, 1, AMOUNT_MSAT(max_msat + 1),
+			     500, &hash, dummy_routing,
+			     NULL, NULL, NULL, NULL, false);
+	assert(e != CHANNEL_ERR_ADD_OK);
+
+	tal_free(dummy_routing);
+}
+
 static struct pubkey pubkey_from_hex(const char *hex)
 {
 	struct pubkey pubkey;
@@ -518,6 +543,9 @@ int main(int argc, const char *argv[])
 				    &local_funding_pubkey,
 				    take(channel_type_static_remotekey(NULL)), false, REMOTE);
 
+	/* Oversized-amount HTLC must be refused, not admitted via overflow. */
+	test_reject_oversized_htlc(lchannel);
+
 	/* BOLT #3:
 	 *
 	 * INTERNAL: local_per_commitment_point: 025f7117a78150fe2ef97db7cfc83bd57b2e2c0d0dd25eaf467a4a1c2a45ce1486

Suggested fix

Enforce the BOLT #1 amount ceiling on all HTLCs before the balance check.

diff --git a/channeld/full_channel.c b/channeld/full_channel.c
--- a/channeld/full_channel.c
+++ b/channeld/full_channel.c
@@ -700,6 +700,20 @@ static enum channel_add_err add_htlc(struct channel *channel,
 		return CHANNEL_ERR_HTLC_BELOW_MINIMUM;
 	}
 
+	/* BOLT #1: milli-satoshi amounts MUST be at most 0x1d24b2dfac520000.
+	 * Enforce this MAX_MONEY bound to avoid overflow in our signed balance
+	 * arithmetic. */
+	if (amount_msat_greater(htlc->amount, AMOUNT_MSAT(0x1d24b2dfac520000ULL))) {
+		return CHANNEL_ERR_MAX_HTLC_VALUE_EXCEEDED;
+	}
+
 	/* FIXME: There used to be a requirement that we not send more than
 	 * 2^32 msat, *but* only electrum enforced it.  Remove in next version:
 	 *

It may also be worthwhile to use a checked subtraction in balance_add_htlc so that any underflow is reported as an error.

Discovery

Found by @dergoegge using smite.

Lenguaje dominante
C
Estrellas
3.1k
Forks
1k
Merge medio
4 d 4 h
PR fusionados (30 d)
27

Preparar el entorno

  • Incluye un Dockerfile o un archivo de Docker Compose
  • Tiene una plantilla de pull request
  • Sin guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de ElementsProject/lightning

Todos los issues de ElementsProject/lightning

Issues similares

Más issues de C

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.