diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index f28f7d6816e925..6db48e604f717c 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -1362,6 +1362,28 @@ def testGetaddrinfo(self): except socket.gaierror: pass + @support.cpython_only + def test_getaddrinfo_invalid_port(self): + # Issue #30711: test that getaddrinfo detects invalid port number + # See HAVE_BROKEN_GETADDRINFO in configure.ac for details. + import _testcapi + flags = getattr(socket, 'AI_NUMERICSERV', None) + with self.assertRaises(socket.gaierror): + socket.getaddrinfo(None, "65536", flags=flags) + with self.assertRaises(socket.gaierror): + socket.getaddrinfo(None, "-1", flags=flags) + with self.assertRaises(socket.gaierror): + socket.getaddrinfo(None, str(_testcapi.LONG_MAX), flags=flags) + with self.assertRaises(socket.gaierror): + socket.getaddrinfo(None, str(_testcapi.LONG_MAX + 1), flags=flags) + with self.assertRaises(socket.gaierror): + socket.getaddrinfo(None, str(_testcapi.LONG_MIN), flags=flags) + with self.assertRaises(socket.gaierror): + socket.getaddrinfo(None, str(_testcapi.LONG_MIN - 1), flags=flags) + with self.assertRaises(socket.gaierror): + socket.getaddrinfo(None, str(_testcapi.ULONG_MAX - 65535 + 1), + flags=flags) + def test_getnameinfo(self): # only IP addresses are allowed self.assertRaises(OSError, socket.getnameinfo, ('mail.python.org',0), 0) diff --git a/Misc/NEWS b/Misc/NEWS index 83eb530602e03d..015dc73d975970 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -1172,6 +1172,8 @@ Library - bpo-30177: path.resolve(strict=False) no longer cuts the path after the first element not present in the filesystem. Patch by Antoine Pietri. +- bpo-30711: socket.getaddrinfo() now detects invalid numeric services. + IDLE ---- diff --git a/Modules/getaddrinfo.c b/Modules/getaddrinfo.c index b6fb53cb3d8d97..f0380da7854ac3 100644 --- a/Modules/getaddrinfo.c +++ b/Modules/getaddrinfo.c @@ -134,7 +134,6 @@ static int get_name(const char *, struct gai_afd *, int); static int get_addr(const char *, int, struct addrinfo **, struct addrinfo *, int); -static int str_isnumber(const char *); static const char * const ai_errlist[] = { "success.", @@ -220,18 +219,6 @@ freeaddrinfo(struct addrinfo *ai) } while ((ai = next) != NULL); } -static int -str_isnumber(const char *p) -{ - unsigned char *q = (unsigned char *)p; - while (*q) { - if (! isdigit(*q)) - return NO; - q++; - } - return YES; -} - int getaddrinfo(const char*hostname, const char*servname, const struct addrinfo *hints, struct addrinfo **res) @@ -333,13 +320,19 @@ getaddrinfo(const char*hostname, const char*servname, * service port */ if (servname) { - if (str_isnumber(servname)) { + long servnum; + char *end; + + servnum = strtol(servname, &end, 10); + if (*end == '\0') { /* treat "" as 0 */ if (pai->ai_socktype == GAI_ANY) { /* caller accept *GAI_ANY* socktype */ pai->ai_socktype = SOCK_DGRAM; pai->ai_protocol = IPPROTO_UDP; } - port = htons((u_short)atoi(servname)); + if (servnum < 0 || servnum > 0xffff) + ERR(EAI_SERVICE); + port = htons((u_short)servnum); } else { struct servent *sp; char *proto; diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 456c66478e9691..ff0978887d6b13 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -375,6 +375,31 @@ const char *inet_ntop(int af, const void *src, char *dst, socklen_t size); #define gai_strerror fake_gai_strerror #define freeaddrinfo fake_freeaddrinfo #include "getaddrinfo.c" +#elif defined(HAVE_BROKEN_GETADDRINFO) +/* getaddrinfo does not detect invalid numeric services. + Check the port number and return EAI_SERVICE if it's + invalid; call getaddrinfo otherwise. */ +static int +wrap_getaddrinfo(const char *hostname, const char *servname, + const struct addrinfo *hints, struct addrinfo **res) +{ + if (servname != NULL && servname[0] != '\0' && + (hints == NULL || /* assume AF_UNSPEC */ + hints->ai_family == AF_UNSPEC || +#ifdef ENABLE_IPV6 + hints->ai_family == AF_INET6 || +#endif + hints->ai_family == AF_INET)) { + long port; + char *end; + + port = strtol(servname, &end, 10); + if (*end == '\0' && (port < 0 || port > 0xffff)) + return EAI_SERVICE; + } + return getaddrinfo(hostname, servname, hints, res); +} +#define getaddrinfo wrap_getaddrinfo #endif #if !defined(HAVE_GETNAMEINFO) #define getnameinfo fake_getnameinfo diff --git a/configure b/configure index ec42e08f8961c1..afaaf51dbdfdaa 100755 --- a/configure +++ b/configure @@ -12991,6 +12991,98 @@ else $as_echo "#define HAVE_GETADDRINFO 1" >>confdefs.h + + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for broken getaddrinfo" >&5 +$as_echo_n "checking for broken getaddrinfo... " >&6; } + if ${ac_cv_broken_getaddrinfo+:} false; then : + $as_echo_n "(cached) " >&6 +else + if test "$cross_compiling" = yes; then : + ac_cv_broken_getaddrinfo=yes +else + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +#include +#include +#include +#include +#include +#include +#include + +static void +invalid_servname(const char *servname) +{ + struct addrinfo hints, *ai; + + memset(&hints, 0, sizeof(hints)); + hints.ai_family = AF_INET; + hints.ai_socktype = 0; + hints.ai_protocol = 0; +#ifdef AI_NUMERICSERV + /* Prevent contacting a resolver if getaddrinfo does not treat + negative numbers as numeric services. */ + hints.ai_flags = AI_NUMERICSERV; +#else + hints.ai_flags = 0; +#endif + + if (getaddrinfo(NULL, servname, &hints, &ai) == 0) { + freeaddrinfo(ai); + exit(1); + } +} + +int main() +{ + char buf[256]; + + invalid_servname("65536"); + invalid_servname("-1"); + + /* Some getaddrinfo functions translate these numeric services to + port number 0 or 65535, depending on whether they use strtol or + strtoul function to convert the service to a number, and whether + or not the sizes of long and int are equal. */ + sprintf(buf, "%ld", LONG_MAX); + invalid_servname(buf); + sprintf(buf, "%lu", ((unsigned long)LONG_MAX) + 1UL); + invalid_servname(buf); + sprintf(buf, "%ld", LONG_MIN); + invalid_servname(buf); + sprintf(buf, "-%lu", ((unsigned long)LONG_MIN) + 1UL); /* LONG_MIN - 1 */ + invalid_servname(buf); + + /* strtoul negates the result if there is a leading minus sign. For + this input, strtoul returns 65535. */ + sprintf(buf, "-%lu", ULONG_MAX - 65535UL + 1UL); + invalid_servname(buf); + + return 0; +} + +_ACEOF +if ac_fn_c_try_run "$LINENO"; then : + ac_cv_broken_getaddrinfo=no +else + ac_cv_broken_getaddrinfo=yes +fi +rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \ + conftest.$ac_objext conftest.beam conftest.$ac_ext +fi + +fi + + + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_broken_getaddrinfo" >&5 +$as_echo "$ac_cv_broken_getaddrinfo" >&6; } + if test $ac_cv_broken_getaddrinfo = yes + then + +$as_echo "#define HAVE_BROKEN_GETADDRINFO 1" >>confdefs.h + + fi fi for ac_func in getnameinfo diff --git a/configure.ac b/configure.ac index 18b940ab329172..b31b35fb340294 100644 --- a/configure.ac +++ b/configure.ac @@ -3992,6 +3992,79 @@ then fi else AC_DEFINE(HAVE_GETADDRINFO, 1, [Define if you have the getaddrinfo function.]) + + AC_MSG_CHECKING(for broken getaddrinfo) + AC_CACHE_VAL(ac_cv_broken_getaddrinfo, + AC_RUN_IFELSE([AC_LANG_SOURCE([[[ +#include +#include +#include +#include +#include +#include +#include + +static void +invalid_servname(const char *servname) +{ + struct addrinfo hints, *ai; + + memset(&hints, 0, sizeof(hints)); + hints.ai_family = AF_INET; + hints.ai_socktype = 0; + hints.ai_protocol = 0; +#ifdef AI_NUMERICSERV + /* Prevent contacting a resolver if getaddrinfo does not treat + negative numbers as numeric services. */ + hints.ai_flags = AI_NUMERICSERV; +#else + hints.ai_flags = 0; +#endif + + if (getaddrinfo(NULL, servname, &hints, &ai) == 0) { + freeaddrinfo(ai); + exit(1); + } +} + +int main() +{ + char buf[256]; + + invalid_servname("65536"); + invalid_servname("-1"); + + /* Some getaddrinfo functions translate these numeric services to + port number 0 or 65535, depending on whether they use strtol or + strtoul function to convert the service to a number, and whether + or not the sizes of long and int are equal. */ + sprintf(buf, "%ld", LONG_MAX); + invalid_servname(buf); + sprintf(buf, "%lu", ((unsigned long)LONG_MAX) + 1UL); + invalid_servname(buf); + sprintf(buf, "%ld", LONG_MIN); + invalid_servname(buf); + sprintf(buf, "-%lu", ((unsigned long)LONG_MIN) + 1UL); /* LONG_MIN - 1 */ + invalid_servname(buf); + + /* strtoul negates the result if there is a leading minus sign. For + this input, strtoul returns 65535. */ + sprintf(buf, "-%lu", ULONG_MAX - 65535UL + 1UL); + invalid_servname(buf); + + return 0; +} + ]]])], + [ac_cv_broken_getaddrinfo=no], + [ac_cv_broken_getaddrinfo=yes], + [ac_cv_broken_getaddrinfo=yes])) + + AC_MSG_RESULT($ac_cv_broken_getaddrinfo) + if test $ac_cv_broken_getaddrinfo = yes + then + AC_DEFINE(HAVE_BROKEN_GETADDRINFO, 1, + [Define if the getaddrinfo function does not detect invalid numeric services.]) + fi fi AC_CHECK_FUNCS(getnameinfo) diff --git a/pyconfig.h.in b/pyconfig.h.in index fa2792b18ad419..941c8f9f01d110 100644 --- a/pyconfig.h.in +++ b/pyconfig.h.in @@ -79,6 +79,10 @@ /* Define to 1 if you have the header file. */ #undef HAVE_BLUETOOTH_H +/* Define if the getaddrinfo function does not detect invalid numeric + services. */ +#undef HAVE_BROKEN_GETADDRINFO + /* Define if mbstowcs(NULL, "text", 0) does not return the number of wide chars that would be converted. */ #undef HAVE_BROKEN_MBSTOWCS