From 9dab20025354c94d620600013247190b181f71fe Mon Sep 17 00:00:00 2001 From: bhuvan-somisetty Date: Wed, 21 Oct 2026 01:18:21 +0530 Subject: [PATCH] common/wireaddr: validate port range in separate_address_and_port and torport `separate_address_and_port()` and `torport=` parsing in `autotor:` and `statictor:` used `strtol()` and assigned the result directly to `u16` port fields. This caused integer truncation modulo 65536, silently accepting out-of-range ports (>65535), negative ports, and signed ports. Use `str_to_u64()` to parse strict unsigned decimal digits and reject ports equal to 0 or greater than `UINT16_MAX`. Changelog-Fixed: common/wireaddr: reject out-of-range (>65535) and negative ports in separate_address_and_port and torport Fixes: #9636 Signed-off-by: bhuvan-somisetty --- common/test/run-ip_port_parsing.c | 15 +++++++++++++++ common/test/run-wireaddr.c | 26 ++++++++++++++++++++++++++ common/wireaddr.c | 26 ++++++++++++++++---------- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/common/test/run-ip_port_parsing.c b/common/test/run-ip_port_parsing.c index 13d34952c50b..a35fdfa749e1 100644 --- a/common/test/run-ip_port_parsing.c +++ b/common/test/run-ip_port_parsing.c @@ -150,7 +150,18 @@ int main(int argc, char *argv[]) assert(!separate_address_and_port(tmpctx, "[::1]:8f", &ip, &port)); assert(!separate_address_and_port(tmpctx, "127.0.0.1:8f", &ip, &port)); assert(!separate_address_and_port(tmpctx, "127.0.0.1:0", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "127.0.0.1:65536", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "127.0.0.1:65537", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "127.0.0.1:75271", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "127.0.0.1:99999", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "127.0.0.1:18446744073709551616", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "127.0.0.1:-1", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "127.0.0.1:+9735", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "127.0.0.1: 9735", &ip, &port)); assert(!separate_address_and_port(tmpctx, "127.0.0.1:ff", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "[::1]:65536", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "[::1]:65537", &ip, &port)); + assert(!separate_address_and_port(tmpctx, "[::1]:-1", &ip, &port)); /* ret = getaddrinfo("[::1]:80", NULL, NULL, &res); */ assert(separate_address_and_port(tmpctx, "[::1]:80", &ip, &port)); @@ -170,6 +181,10 @@ int main(int argc, char *argv[]) assert(streq(ip, "192.168.1.1")); assert(port == 8000); + assert(separate_address_and_port(tmpctx, "127.0.0.1:65535", &ip, &port)); + assert(streq(ip, "127.0.0.1")); + assert(port == 65535); + port = 0; assert(separate_address_and_port(tmpctx, "192.168.2.255", &ip, &port)); assert(streq(ip, "192.168.2.255")); diff --git a/common/test/run-wireaddr.c b/common/test/run-wireaddr.c index c94d451c4e49..173d1895d36d 100644 --- a/common/test/run-wireaddr.c +++ b/common/test/run-wireaddr.c @@ -126,6 +126,19 @@ int main(int argc, char *argv[]) assert(parse_wireaddr(tmpctx, "127.0.0.1", 9055, NULL, &expect->u.torservice.address) == NULL); assert(wireaddr_internal_eq(&addr, expect)); + /* autotor address with valid boundary torport */ + assert(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1/torport=65535", DEFAULT_PORT, false, &addr) == NULL); + assert(addr.u.torservice.port == 65535); + + /* autotor address with invalid torports */ + assert(streq(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1/torport=0", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1/torport=65536", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1/torport=65537", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1/torport=75271", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1/torport=-1", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1/torport=+9735", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "autotor:127.0.0.1/torport= 9735", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + /* statictor address */ assert(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1", DEFAULT_PORT, false, &addr) == NULL); expect->itype = ADDR_INTERNAL_STATICTOR; @@ -165,6 +178,19 @@ int main(int argc, char *argv[]) assert(parse_wireaddr(tmpctx, "127.0.0.1", 9055, NULL, &expect->u.torservice.address) == NULL); assert(wireaddr_internal_eq(&addr, expect)); + /* statictor address with valid boundary torport */ + assert(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1/torport=65535", DEFAULT_PORT, false, &addr) == NULL); + assert(addr.u.torservice.port == 65535); + + /* statictor address with invalid torports */ + assert(streq(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1/torport=0", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1/torport=65536", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1/torport=65537", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1/torport=75271", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1/torport=-1", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1/torport=+9735", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + assert(streq(parse_wireaddr_internal(tmpctx, "statictor:127.0.0.1/torport= 9735", DEFAULT_PORT, false, &addr), "Bad :torport: number")); + /* local socket path */ assert(parse_wireaddr_internal(tmpctx, "/tmp/foo.sock", DEFAULT_PORT, false, &addr) == NULL); expect->itype = ADDR_INTERNAL_SOCKNAME; diff --git a/common/wireaddr.c b/common/wireaddr.c index 1364e4931383..67e0e1422b13 100644 --- a/common/wireaddr.c +++ b/common/wireaddr.c @@ -344,9 +344,13 @@ bool separate_address_and_port(const tal_t *ctx, const char *arg, } if (portcolon) { - char *endp; - *port = strtol(portcolon + 1, &endp, 10); - return *port != 0 && *endp == '\0'; + u64 p; + if (!str_to_u64(portcolon + 1, strlen(portcolon + 1), &p)) + return false; + if (p == 0 || p > UINT16_MAX) + return false; + *port = p; + return true; } return true; @@ -671,11 +675,12 @@ const char *parse_wireaddr_internal(const tal_t *ctx, for (size_t i = 1; i < tal_count(parts)-1; i++) { if (strstarts(parts[i], "torport=")) { - char *endp = NULL; - addr->u.torservice.port = strtol(parts[i]+strlen("torport="), &endp, 10); - if (addr->u.torservice.port <= 0 || *endp != '\0') { + const char *val = parts[i] + strlen("torport="); + u64 p; + if (!str_to_u64(val, strlen(val), &p) || p == 0 || p > UINT16_MAX) { return "Bad :torport: number"; } + addr->u.torservice.port = p; } else { return tal_fmt(ctx, "unknown tor arg %s", parts[i]); } @@ -699,11 +704,12 @@ const char *parse_wireaddr_internal(const tal_t *ctx, char **parts = tal_strsplit(tmpctx, arg, "/", STR_EMPTY_OK); for (size_t i = 1; i < tal_count(parts)-1; i++) { if (strstarts(parts[i], "torport=")) { - char *endp = NULL; - addr->u.torservice.port = strtol(parts[i]+strlen("torport="), &endp, 10); - if (addr->u.torservice.port <= 0 || *endp != '\0') { - return "Bad :torport: number"; + const char *val = parts[i] + strlen("torport="); + u64 p; + if (!str_to_u64(val, strlen(val), &p) || p == 0 || p > UINT16_MAX) { + return "Bad :torport: number"; } + addr->u.torservice.port = p; } else if (strstarts(parts[i], "torblob=")) { const char *blobdata = parts[i] + strlen("torblob="); if (strlen(blobdata) > TOR_V3_BLOBLEN) {