python · smejkar · Jun 27, 2017 · Jun 27, 2017 · Jun 27, 2017 · Jun 27, 2017
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 <sys/types.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <limits.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+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
@@ -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 <sys/types.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <limits.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+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
@@ -79,6 +79,10 @@
 /* Define to 1 if you have the <bluetooth.h> 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