common: correctly refuse to accept wireaddr with port == 0.
What the developer wrote
Authored by Rusty Russell
common: correctly refuse to accept wireaddr with port == 0.
BOLT 7 says this, but we don't do it. (Actually, it only says that for certain types,
but I've fixed that in https://github.com/lightning/bolts/pull/1331).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This change tightens how Core Lightning handles network addresses it receives from other nodes. Previously, the code would accept an address even if its port number was 0. The patch now treats port 0 as a signal to ignore the address, matching the Lightning protocol specification (BOLT 7). It also improves error reporting so callers can tell the difference between a malformed address, an unknown address type, and an address that should simply be skipped. The main risk is that a peer could announce a useless port-0 address and, before this fix, the node might have tried to use it in ways that caused confusion or minor failures.
Treat as a low-to-moderate hardening patch. Review whether any other callers outside the changed files still use fromwire_wireaddr() with a boolean check, and update them. Consider adding explicit tests for port-0 wireaddr handling and for malformed/unknown/ignore return-code paths. No urgent security response appears required based solely on this diff.
Security signals we found
Protocol conformance fix: rejects/ignores wireaddr with port == 0 per BOLT 7
API change from boolean to explicit return codes improves failure distinguishability
Potential DoS/harmless peer behavior: accepting port-0 addresses could lead to storing or attempting to connect to invalid endpoints
No explicit memory safety, cryptographic, or remote-code-execution signals in the diff
Evidence from the diff
fromwire_wireaddr() in common/wireaddr.c is changed from returning bool to returning an enum: FROMWIREADDR_MALFORMED, FROMWIREADDR_UNKNOWN, FROMWIREADDR_IGNORE, or FROMWIREADDR_OK. The key new behavior is that any successfully parsed wireaddr with port == 0 now returns FROMWIREADDR_IGNORE instead of FROMWIREADDR_OK. Callers are updated to check for FROMWIREADDR_OK explicitly. fromwire_wireaddr_array() now skips ignored entries rather than adding them to the result array, and treats unknown address types as a stop condition with len=0. Internal address parsing for AUTOTOR/STATICTOR/WIREADDR now fails the cursor if the embedded wireaddr is not OK. The change aligns with BOLT 7’s guidance to ignore addresses with port 0.
Changed components
common/wireaddr.ccommon/wireaddr.hcommon/test/run-wireaddr.cconnectd/peer_exchange_initmsg.cdb/bindings.cdevtools/print_wire.ctests/fuzz/fuzz-wireaddr.cInspect captured patch +49 / −21
diff --git a/common/test/run-wireaddr.c b/common/test/run-wireaddr.c
index 6e8ccda4..5c10d191 100644
--- a/common/test/run-wireaddr.c
+++ b/common/test/run-wireaddr.c
@@ -260,7 +260,7 @@ int main(int argc, char *argv[])
size_t encoded_wa_len = tal_bytelen(encoded_wa);
struct wireaddr decoded_wa;
- assert(fromwire_wireaddr((const u8 **) &encoded_wa, &encoded_wa_len, &decoded_wa));
+ assert(fromwire_wireaddr((const u8 **) &encoded_wa, &encoded_wa_len, &decoded_wa) == FROMWIREADDR_OK);
assert(wireaddr_eq(&wa, &decoded_wa));
tal_free(expect);
diff --git a/common/wireaddr.c b/common/wireaddr.c
index c4e840ea..80932566 100644
--- a/common/wireaddr.c
+++ b/common/wireaddr.c
@@ -27,10 +27,11 @@ bool wireaddr_eq_without_port(const struct wireaddr *a, const struct wireaddr *b
return memeq(a->addr, a->addrlen, b->addr, b->addrlen);
}
-/* Returns false if we didn't parse it, and *cursor == NULL if malformed. */
-bool fromwire_wireaddr(const u8 **cursor, size_t *max, struct wireaddr *addr)
+enum fromwireaddr_ret fromwire_wireaddr(const u8 **cursor, size_t *max, struct wireaddr *addr)
{
addr->type = fromwire_u8(cursor, max);
+ if (*cursor == NULL)
+ return FROMWIREADDR_MALFORMED;
switch (addr->type) {
case ADDR_TYPE_IPV4:
@@ -50,12 +51,21 @@ bool fromwire_wireaddr(const u8 **cursor, size_t *max, struct wireaddr *addr)
memset(&addr->addr, 0, sizeof(addr->addr));
break;
default:
- return false;
+ return FROMWIREADDR_UNKNOWN;
}
fromwire(cursor, max, addr->addr, addr->addrlen);
addr->port = fromwire_u16(cursor, max);
- return *cursor != NULL;
+ if (*cursor == NULL)
+ return FROMWIREADDR_MALFORMED;
+ /* BOLT #7:
+ * - if `port` is equal to 0:
+ * - SHOULD ignore `ipv6_addr` OR `ipv4_addr` OR `hostname`.
+ */
+ /* FIXME: This seems universal? */
+ if (addr->port == 0)
+ return FROMWIREADDR_IGNORE;
+ return FROMWIREADDR_OK;
}
void towire_wireaddr(u8 **pptr, const struct wireaddr *addr)
@@ -130,18 +140,20 @@ bool fromwire_wireaddr_internal(const u8 **cursor, size_t *max,
addr->u.allproto.port = fromwire_u16(cursor, max);
return *cursor != NULL;
case ADDR_INTERNAL_AUTOTOR:
- fromwire_wireaddr(cursor, max, &addr->u.torservice.address);
+ if (fromwire_wireaddr(cursor, max, &addr->u.torservice.address) != FROMWIREADDR_OK)
+ fromwire_fail(cursor, max);
addr->u.torservice.port = fromwire_u16(cursor, max);
return *cursor != NULL;
case ADDR_INTERNAL_STATICTOR:
- fromwire_wireaddr(cursor, max, &addr->u.torservice.address);
+ if (fromwire_wireaddr(cursor, max, &addr->u.torservice.address) != FROMWIREADDR_OK)
+ fromwire_fail(cursor, max);
fromwire_u8_array(cursor, max, (u8 *)addr->u.torservice.blob,
sizeof(addr->u.torservice.blob));
addr->u.torservice.port = fromwire_u16(cursor, max);
return *cursor != NULL;
case ADDR_INTERNAL_WIREADDR:
addr->u.wireaddr.is_websocket = fromwire_bool(cursor, max);
- return fromwire_wireaddr(cursor, max, &addr->u.wireaddr.wireaddr);
+ return fromwire_wireaddr(cursor, max, &addr->u.wireaddr.wireaddr) == FROMWIREADDR_OK;
case ADDR_INTERNAL_FORPROXY:
fromwire_u8_array(cursor, max, (u8 *)addr->u.unresolved.name,
sizeof(addr->u.unresolved.name));
@@ -859,15 +871,21 @@ struct wireaddr *fromwire_wireaddr_array(const tal_t *ctx, const u8 *ser)
* - SHOULD ignore the first `address descriptor` that does
* NOT match the types defined above.
*/
- if (!fromwire_wireaddr(&cursor, &len, &wireaddr)) {
- if (!cursor)
- /* Parsing address failed */
- return tal_free(wireaddrs);
+ switch (fromwire_wireaddr(&cursor, &len, &wireaddr)) {
+ case FROMWIREADDR_MALFORMED:
+ /* Parsing address failed */
+ return tal_free(wireaddrs);
+ case FROMWIREADDR_UNKNOWN:
/* Unknown type, stop there. */
- break;
+ len = 0;
+ continue;
+ case FROMWIREADDR_IGNORE:
+ continue;
+ case FROMWIREADDR_OK:
+ tal_arr_expand(&wireaddrs, wireaddr);
+ continue;
}
-
- tal_arr_expand(&wireaddrs, wireaddr);
+ abort();
}
return wireaddrs;
}
diff --git a/common/wireaddr.h b/common/wireaddr.h
index efa0e1f1..2031aec9 100644
--- a/common/wireaddr.h
+++ b/common/wireaddr.h
@@ -60,8 +60,15 @@ enum addr_listen_announce {
ADDR_LISTEN_AND_ANNOUNCE = ADDR_LISTEN|ADDR_ANNOUNCE
};
+enum fromwireaddr_ret {
+ FROMWIREADDR_MALFORMED,
+ FROMWIREADDR_UNKNOWN,
+ FROMWIREADDR_IGNORE,
+ FROMWIREADDR_OK,
+};
+
void towire_wireaddr(u8 **pptr, const struct wireaddr *addr);
-bool fromwire_wireaddr(const u8 **cursor, size_t *max, struct wireaddr *addr);
+enum fromwireaddr_ret fromwire_wireaddr(const u8 **cursor, size_t *max, struct wireaddr *addr);
enum addr_listen_announce fromwire_addr_listen_announce(const u8 **cursor,
size_t *max);
diff --git a/connectd/peer_exchange_initmsg.c b/connectd/peer_exchange_initmsg.c
index a7321369..eb7bec64 100644
--- a/connectd/peer_exchange_initmsg.c
+++ b/connectd/peer_exchange_initmsg.c
@@ -117,7 +117,7 @@ static struct io_plan *peer_init_received(struct io_conn *conn,
size_t len = tal_bytelen(tlvs->remote_addr);
remote_addr = tal(peer, struct wireaddr);
- if (fromwire_wireaddr(&cursor, &len, remote_addr)) {
+ if (fromwire_wireaddr(&cursor, &len, remote_addr) == FROMWIREADDR_OK) {
switch (remote_addr->type) {
case ADDR_TYPE_IPV4:
case ADDR_TYPE_IPV6:
diff --git a/db/bindings.c b/db/bindings.c
index 8dac3ea6..d61fca9f 100644
--- a/db/bindings.c
+++ b/db/bindings.c
@@ -614,7 +614,7 @@ struct wireaddr *db_col_wireaddr(const tal_t *ctx,
struct wireaddr *waddr = tal(ctx, struct wireaddr);
const u8 *wire = db_col_arr(tmpctx, stmt, colname, u8);
size_t len = tal_bytelen(wire);
- if (!fromwire_wireaddr(&wire, &len, waddr))
+ if (fromwire_wireaddr(&wire, &len, waddr) != FROMWIREADDR_OK)
return tal_free(waddr);
return waddr;
}
diff --git a/devtools/print_wire.c b/devtools/print_wire.c
index a99a4955..9afd8f7e 100644
--- a/devtools/print_wire.c
+++ b/devtools/print_wire.c
@@ -135,7 +135,10 @@ bool printwire_tu64(const char *fieldname, const u8 **cursor, size_t *plen)
bool printwire_wireaddr(const char *fieldname, const u8 **cursor, size_t *plen)
{
struct wireaddr w;
- if (!fromwire_wireaddr(cursor, plen, &w))
+ enum fromwireaddr_ret r;
+
+ r = fromwire_wireaddr(cursor, plen, &w);
+ if (r != FROMWIREADDR_OK && r != FROMWIREADDR_IGNORE)
return false;
printf("%s\n", fmt_wireaddr(tmpctx, &w));
return true;
@@ -206,7 +209,7 @@ static bool printwire_addresses(const u8 **cursor, size_t *plen, size_t len)
const size_t len_ref = *plen;
printf("[");
- while (to_go && fromwire_wireaddr(cursor, plen, &addr)) {
+ while (to_go && fromwire_wireaddr(cursor, plen, &addr) == FROMWIREADDR_OK) {
to_go = len - (len_ref - *plen);
printf(" %s", fmt_wireaddr(NULL, &addr));
}
diff --git a/tests/fuzz/fuzz-wireaddr.c b/tests/fuzz/fuzz-wireaddr.c
index 32d480ba..cd5c4d10 100644
--- a/tests/fuzz/fuzz-wireaddr.c
+++ b/tests/fuzz/fuzz-wireaddr.c
@@ -34,7 +34,7 @@ void run(const uint8_t *data, size_t size)
size_t len = tal_bytelen(output_buffer);
assert(fromwire_wireaddr((const u8 **)&output_buffer, &len,
- &decoded_wa));
+ &decoded_wa) == FROMWIREADDR_OK);
assert(wireaddr_eq(&wa, &decoded_wa));
}
Why this scored 51/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.