From: Juergen Perlinger Date: Wed, 20 May 2020 07:44:15 +0000 (+0200) Subject: [Bug 3667] decodenetnum fails with numeric port X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=44a7fa70513b3d0131f272007dca4991c843afcf;p=thirdparty%2Fntp.git [Bug 3667] decodenetnum fails with numeric port bk: 5ec4dfcf5ZlE0vEuAccGg2lJpsZEOw --- diff --git a/ChangeLog b/ChangeLog index 0e1ccc200..d26aed585 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +--- +* [Bug 3667] decodenetnum fails with numeric port + - rewrite 'decodenetnum()' in terms of inet_pton + --- (4.2.8p15) 2020/04/xx Released by Harlan Stenn diff --git a/configure.ac b/configure.ac index a0072eeb5..5dc6aee02 100644 --- a/configure.ac +++ b/configure.ac @@ -912,7 +912,7 @@ case "$host" in ;; esac AC_CHECK_FUNCS([setlinebuf setpgid setpriority setsid setvbuf]) -AC_CHECK_FUNCS([strdup strerror setrlimit strchr]) +AC_CHECK_FUNCS([strdup strnlen memchr strerror setrlimit strchr]) case "$host" in *-*-aix[[4-9]]*) # XXX only verified thru AIX6. diff --git a/include/l_stdlib.h b/include/l_stdlib.h index 073ea4641..fbf57c53a 100644 --- a/include/l_stdlib.h +++ b/include/l_stdlib.h @@ -221,4 +221,13 @@ extern int errno; extern int h_errno; #endif +#ifndef HAVE_MEMCHR +extern void *memchr(const void *s, int c, size_t n); +#endif + +#ifndef HAVE_STRNLEN +extern size_t strnlen(const char *s, size_t n); +#endif + + #endif /* L_STDLIB_H */ diff --git a/libntp/decodenetnum.c b/libntp/decodenetnum.c index 35e839aaf..d8a7296e4 100644 --- a/libntp/decodenetnum.c +++ b/libntp/decodenetnum.c @@ -13,106 +13,152 @@ #include "ntp.h" #include "ntp_stdlib.h" -#include "ntp_assert.h" -#define PORTSTR(x) _PORTSTR(x) -#define _PORTSTR(x) #x -static int -isnumstr( - const char *s +/* If the given string position points to a decimal digit, parse the + * number. If this is not possible, or the parsing did not consume the + * whole string, or if the result exceeds the maximum value, return the + * default value. + */ +static unsigned long +_num_or_dflt( + char * sval, + unsigned long maxval, + unsigned long defval ) { - while (*s >= '0' && *s <= '9') - ++s; - return !*s; + char * ep; + unsigned long num; + + if (!(sval && isdigit(*(unsigned char*)sval))) + return defval; + + num = strtoul(sval, &ep, 10); + if (!*ep && num <= maxval) + return num; + + return defval; +} + +/* If the given string position is not NULL and does not point to the + * terminator, replace the character with NUL and advance the pointer. + * Return the resulting position. + */ +static inline char* +_chop( + char * sp) +{ + if (sp && *sp) + *sp++ = '\0'; + return sp; +} + +/* If the given string position points to the given char, advance the + * pointer and return the result. Otherwise, return NULL. + */ +static inline char* +_skip( + char * sp, + int ch) +{ + if (sp && *(unsigned char*)sp == ch) + return (sp + 1); + return NULL; } /* * decodenetnum convert text IP address and port to sockaddr_u * - * Returns 0 for failure, 1 for success. + * Returns FALSE (->0) for failure, TRUE (->1) for success. */ int decodenetnum( const char *num, - sockaddr_u *netnum + sockaddr_u *net ) { - static const char * const servicename = "ntp"; - static const char * const serviceport = PORTSTR(NTP_PORT); + /* Building a parser is more fun in Haskell, but here we go... + * + * This works through 'inet_pton()' taking the brunt of the + * work, after some string manipulations to split off URI + * brackets, ports and scope identifiers. The heuristics are + * simple but must hold for all _VALID_ addresses. inet_pton() + * will croak on bad ones later, but replicating the whole + * parser logic to detect errors is wasteful. + */ - struct addrinfo hints, *ai = NULL; - int err; - const char *host_str; - const char *port_str; - char *pp; - char *np; - char nbuf[80]; - - REQUIRE(num != NULL); - - if (strlen(num) >= sizeof(nbuf)) { - printf("length error\n"); + sockaddr_u netnum; + char buf[64]; /* working copy of input */ + char *haddr=buf; + unsigned int port=NTP_PORT, scope=0; + sa_family_t afam=AF_UNSPEC; + + /* copy input to working buffer with length check */ + if (strlcpy(buf, num, sizeof(buf)) >= sizeof(buf)) return FALSE; - } - port_str = servicename; - if ('[' != num[0]) { - /* - * to distinguish IPv6 embedded colons from a port - * specification on an IPv4 address, assume all - * legal IPv6 addresses have at least two colons. - */ - pp = strchr(num, ':'); - if (NULL == pp) - host_str = num; /* no colons */ - else if (NULL != strchr(pp + 1, ':')) - host_str = num; /* two or more colons */ - else { /* one colon */ - strlcpy(nbuf, num, sizeof(nbuf)); - host_str = nbuf; - pp = strchr(nbuf, ':'); - *pp = '\0'; - port_str = pp + 1; + /* Identify address family and possibly the port, if given. If + * this results in AF_UNSPEC, we will fail in the next step. + */ + if (*haddr == '[') { + char * endp = strchr(++haddr, ']'); + if (endp) { + port = _num_or_dflt(_skip(_chop(endp), ':'), + 0xFFFFu, port); + afam = strchr(haddr, ':') ? AF_INET6 : AF_INET; } } else { - host_str = np = nbuf; - while (*++num && ']' != *num) - *np++ = *num; - *np = 0; - if (']' == num[0] && ':' == num[1] && '\0' != num[2]) - port_str = &num[2]; + char *col = strchr(haddr, ':'); + char *dot = strchr(haddr, '.'); + if (col == dot) { + /* no dot, no colon: bad! */ + afam = AF_UNSPEC; + } else if (!col) { + /* no colon, only dot: IPv4! */ + afam = AF_INET; + } else if (!dot || col < dot) { + /* no dot or 1st colon before 1st dot: IPv6! */ + afam = AF_INET6; + } else { + /* 1st dot before 1st colon: must be IPv4 with port */ + afam = AF_INET; + port = _num_or_dflt(_chop(col), 0xFFFFu, port); + } } - if ( ! *host_str) + + /* Since we don't know about additional members in the address + * structures, we wipe the result buffer thoroughly: + */ + memset(&netnum, 0, sizeof(netnum)); + + /* For AF_INET6, evaluate and remove any scope suffix. Have + * inet_pton() do the real work for AF_INET and AF_INET6, bail + * out otherwise: + */ + switch (afam) { + case AF_INET: + if (inet_pton(afam, haddr, &netnum.sa4.sin_addr) <= 0) + return FALSE; + netnum.sa4.sin_port = htons((unsigned short)port); + break; + + case AF_INET6: + scope = _num_or_dflt(_chop(strchr(haddr, '%')), 0xFFFFFFFFu, scope); + if (inet_pton(afam, haddr, &netnum.sa6.sin6_addr) <= 0) + return FALSE; + netnum.sa6.sin6_port = htons((unsigned short)port); + netnum.sa6.sin6_scope_id = scope; + break; + + case AF_UNSPEC: + default: return FALSE; - if ( ! *port_str) - port_str = servicename; - - ZERO(hints); - hints.ai_flags |= Z_AI_NUMERICHOST; - if (isnumstr(port_str)) - hints.ai_flags |= Z_AI_NUMERICSERV; - err = getaddrinfo(host_str, port_str, &hints, &ai); - /* retry with default service name if the service lookup failed */ - if (err == EAI_SERVICE && strcmp(port_str, servicename)) { - hints.ai_flags &= ~Z_AI_NUMERICSERV; - port_str = servicename; - err = getaddrinfo(host_str, port_str, &hints, &ai); } - /* retry another time with default service port if the service lookup failed */ - if (err == EAI_SERVICE && strcmp(port_str, serviceport)) { - hints.ai_flags |= Z_AI_NUMERICSERV; - port_str = serviceport; - err = getaddrinfo(host_str, port_str, &hints, &ai); - } - if (err != 0) - return FALSE; - - INSIST(ai->ai_addrlen <= sizeof(*netnum)); - ZERO(*netnum); - memcpy(netnum, ai->ai_addr, ai->ai_addrlen); - freeaddrinfo(ai); + /* Collect the remaining pieces and feed the output, which was + * not touched so far: + */ + netnum.sa.sa_family = afam; + memcpy(net, &netnum, sizeof(netnum)); return TRUE; } diff --git a/libntp/strdup.c b/libntp/strdup.c index 62d5a16d4..5d9d73876 100644 --- a/libntp/strdup.c +++ b/libntp/strdup.c @@ -3,11 +3,13 @@ #include #include "ntp_malloc.h" #include +#include "l_stdlib.h" -#ifndef HAVE_STRDUP +#define STRDUP_EMPTY_UNIT +#ifndef HAVE_STRDUP +# undef STRDUP_EMPTY_UNIT char *strdup(const char *s); - char * strdup( const char *s @@ -24,6 +26,30 @@ strdup( return cp; } -#else +#endif + +#ifndef HAVE_MEMCHR +# undef STRDUP_EMPTY_UNIT +void *memchr(const void *s, int c, size_t n) +{ + const unsignec char *p = s; + while (n && *p != c) { + --n; + ++p; + } + return n ? p : NULL; +} +#endif + +#ifndef HAVE_STRNLEN +# undef STRDUP_EMPTY_UNIT +size_t strnlen(const char *s, size_t n) +{ + const char *e = memchr(s, 0, n); + return e ? (size_t)(e - s) : n; +} +#endif + +#ifdef STRDUP_EMPTY_UNIT int strdup_c_nonempty_compilation_unit; #endif diff --git a/tests/libntp/decodenetnum.c b/tests/libntp/decodenetnum.c index 85463e868..961ea6d30 100644 --- a/tests/libntp/decodenetnum.c +++ b/tests/libntp/decodenetnum.c @@ -9,8 +9,11 @@ extern void test_IPv4AddressOnly(void); extern void test_IPv4AddressWithPort(void); extern void test_IPv6AddressOnly(void); extern void test_IPv6AddressWithPort(void); +extern void test_IPv6AddressWithScope(void); +extern void test_IPv6AddressWithPortAndScope(void); extern void test_IllegalAddress(void); extern void test_IllegalCharInPort(void); +extern void test_NameBufOverflow(void); /* * NOTE: The IPv6 specific tests are reduced to stubs when IPv6 is @@ -35,6 +38,7 @@ test_IPv4AddressOnly(void) sockaddr_u actual; sockaddr_u expected; + memset(&expected, 0, sizeof(expected)); expected.sa4.sin_family = AF_INET; expected.sa4.sin_addr.s_addr = inet_addr("192.0.2.1"); SET_PORT(&expected, NTP_PORT); @@ -50,6 +54,7 @@ test_IPv4AddressWithPort(void) sockaddr_u actual; sockaddr_u expected; + memset(&expected, 0, sizeof(expected)); expected.sa4.sin_family = AF_INET; expected.sa4.sin_addr.s_addr = inet_addr("192.0.2.2"); SET_PORT(&expected, 2000); @@ -71,21 +76,26 @@ test_IPv6AddressOnly(void) 0x03, 0x70, 0x73, 0x34 }; - const char *str = "2001:0db8:85a3:08d3:1319:8a2e:0370:7334"; + const char *str1 = "2001:0db8:85a3:08d3:1319:8a2e:0370:7334"; + const char *str2 = "[2001:0db8:85a3:08d3:1319:8a2e:0370:7334]"; sockaddr_u actual; sockaddr_u expected; + memset(&expected, 0, sizeof(expected)); expected.sa6.sin6_family = AF_INET6; expected.sa6.sin6_addr = address; SET_PORT(&expected, NTP_PORT); - TEST_ASSERT_TRUE(decodenetnum(str, &actual)); + TEST_ASSERT_TRUE(decodenetnum(str1, &actual)); + TEST_ASSERT_TRUE(IsEqual(expected, actual)); + + TEST_ASSERT_TRUE(decodenetnum(str2, &actual)); TEST_ASSERT_TRUE(IsEqual(expected, actual)); #else - + TEST_IGNORE_MESSAGE("IPV6 disabled in build"); - + #endif } @@ -106,6 +116,7 @@ test_IPv6AddressWithPort(void) sockaddr_u actual; sockaddr_u expected; + memset(&expected, 0, sizeof(expected)); expected.sa6.sin6_family = AF_INET6; expected.sa6.sin6_addr = address; SET_PORT(&expected, 3000); @@ -114,12 +125,77 @@ test_IPv6AddressWithPort(void) TEST_ASSERT_TRUE(IsEqual(expected, actual)); #else - + + TEST_IGNORE_MESSAGE("IPV6 disabled in build"); + +#endif +} + +void test_IPv6AddressWithScope(void) +{ +#if defined(ISC_PLATFORM_HAVEIPV6) && defined(WANT_IPV6) + + const struct in6_addr address = { + 0x20, 0x01, 0x0d, 0xb8, + 0x85, 0xa3, 0x08, 0xd3, + 0x13, 0x19, 0x8a, 0x2e, + 0x03, 0x70, 0x73, 0x34 + }; + + const char *str1 = "2001:0db8:85a3:08d3:1319:8a2e:0370:7334%42"; + const char *str2 = "[2001:0db8:85a3:08d3:1319:8a2e:0370:7334%42]"; + sockaddr_u actual; + + sockaddr_u expected; + memset(&expected, 0, sizeof(expected)); + expected.sa6.sin6_family = AF_INET6; + expected.sa6.sin6_addr = address; + expected.sa6.sin6_scope_id = 42; + SET_PORT(&expected, NTP_PORT); + + TEST_ASSERT_TRUE(decodenetnum(str1, &actual)); + TEST_ASSERT_TRUE(IsEqual(expected, actual)); + + TEST_ASSERT_TRUE(decodenetnum(str2, &actual)); + TEST_ASSERT_TRUE(IsEqual(expected, actual)); + +#else + TEST_IGNORE_MESSAGE("IPV6 disabled in build"); - + #endif } +void test_IPv6AddressWithPortAndScope(void) +{ +#if defined(ISC_PLATFORM_HAVEIPV6) && defined(WANT_IPV6) + + const struct in6_addr address = { + 0x20, 0x01, 0x0d, 0xb8, + 0x85, 0xa3, 0x08, 0xd3, + 0x13, 0x19, 0x8a, 0x2e, + 0x03, 0x70, 0x73, 0x34 + }; + + const char *str = "[2001:0db8:85a3:08d3:1319:8a2e:0370:7334%42]:3000"; + sockaddr_u actual; + + sockaddr_u expected; + memset(&expected, 0, sizeof(expected)); + expected.sa6.sin6_family = AF_INET6; + expected.sa6.sin6_addr = address; + expected.sa6.sin6_scope_id = 42; + SET_PORT(&expected, 3000); + + TEST_ASSERT_TRUE(decodenetnum(str, &actual)); + TEST_ASSERT_TRUE(IsEqual(expected, actual)); + +#else + + TEST_IGNORE_MESSAGE("IPV6 disabled in build"); + +#endif +} void test_IllegalAddress(void) @@ -141,6 +217,7 @@ test_IllegalCharInPort(void) sockaddr_u actual; sockaddr_u expected; + memset(&expected, 0, sizeof(expected)); expected.sa4.sin_family = AF_INET; expected.sa4.sin_addr.s_addr = inet_addr("192.0.2.1"); SET_PORT(&expected, NTP_PORT); @@ -148,3 +225,14 @@ test_IllegalCharInPort(void) TEST_ASSERT_TRUE(decodenetnum(str, &actual)); TEST_ASSERT_TRUE(IsEqual(expected, actual)); } + +void +test_NameBufOverflow(void) +{ + const char *str = + "loremipsumloremipsumloremipsumloremipsumloremipsum" + "loremipsumloremipsumloremipsumloremipsum"; + + sockaddr_u actual; + TEST_ASSERT_FALSE(decodenetnum(str, &actual)); +} diff --git a/tests/libntp/netof.c b/tests/libntp/netof.c index 59dd7098d..be197cf18 100644 --- a/tests/libntp/netof.c +++ b/tests/libntp/netof.c @@ -89,11 +89,13 @@ test_IPv6Address(void) } } }; // 2001:0db8:85a3:08d3:0000:0000:0000:0000 sockaddr_u input; + memset(&input, 0, sizeof(input)); input.sa6.sin6_family = AF_INET6; input.sa6.sin6_addr = input_address; SET_PORT(&input, 3000); sockaddr_u expected; + memset(&expected, 0, sizeof(expected)); expected.sa6.sin6_family = AF_INET6; expected.sa6.sin6_addr = expected_address; SET_PORT(&expected, 3000); diff --git a/tests/libntp/run-decodenetnum.c b/tests/libntp/run-decodenetnum.c index ef2b3c68e..0d3064972 100644 --- a/tests/libntp/run-decodenetnum.c +++ b/tests/libntp/run-decodenetnum.c @@ -33,8 +33,11 @@ extern void test_IPv4AddressOnly(void); extern void test_IPv4AddressWithPort(void); extern void test_IPv6AddressOnly(void); extern void test_IPv6AddressWithPort(void); +extern void test_IPv6AddressWithScope(void); +extern void test_IPv6AddressWithPortAndScope(void); extern void test_IllegalAddress(void); extern void test_IllegalCharInPort(void); +extern void test_NameBufOverflow(void); //=======Suite Setup===== @@ -67,8 +70,11 @@ int main(int argc, char *argv[]) RUN_TEST(test_IPv4AddressWithPort, 9); RUN_TEST(test_IPv6AddressOnly, 10); RUN_TEST(test_IPv6AddressWithPort, 11); - RUN_TEST(test_IllegalAddress, 12); - RUN_TEST(test_IllegalCharInPort, 13); + RUN_TEST(test_IPv6AddressWithScope, 12); + RUN_TEST(test_IPv6AddressWithPortAndScope, 13); + RUN_TEST(test_IllegalAddress, 14); + RUN_TEST(test_IllegalCharInPort, 15); + RUN_TEST(test_NameBufOverflow, 16); return (UnityEnd()); } diff --git a/tests/libntp/sockaddrtest.c b/tests/libntp/sockaddrtest.c index bbf669c78..f7c2206ec 100644 --- a/tests/libntp/sockaddrtest.c +++ b/tests/libntp/sockaddrtest.c @@ -42,7 +42,8 @@ IsEqual(const sockaddr_u expected, const sockaddr_u actual) { } } else if (actual.sa.sa_family == AF_INET6) { //IPv6 if (expected.sa6.sin6_port == actual.sa6.sin6_port && - memcmp(&expected.sa6.sin6_addr, &actual.sa6.sin6_addr, + expected.sa6.sin6_scope_id == actual.sa6.sin6_scope_id && + memcmp(&expected.sa6.sin6_addr, &actual.sa6.sin6_addr, sizeof(in6)) == 0) { return TRUE; } else {