From a81420d038647c17c3e20eaf2a8a1bca761b6755 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mikl=C3=B3s=20M=C3=A1t=C3=A9?= <mtmkls@gmail.com>
Date: Sat, 17 Aug 2024 10:07:12 +0200
Subject: [PATCH] [v2] nss: fix getaddrinfo() accepting garbage as valid IPv4
address
To: libc-alpha@sourceware.org
Using inet_aton() for numeric addresses is not a good idea, because it
accepts non-RFC-compliant strings, like "1", "1.2", "1.2.3", "123456",
"0xbeef" etc. as a valid IPv4 address, and this behavior is even documented
in its man page. Note that when .ai_family=AF_INET, and the numeric address
decoding fails, in the next step getaddrinfo() calls gethostbyname(), which
tries to decode it as numeric again (see digits_dots.c), so gethostbyname()
must be fixed as well.
I tested getaddrinfo() on other systems:
- on FreeBSD it's broken like in glibc
- on Windows the WinSock library only accepts RFC-compliant addresses
This patch also includes a new test case in test 3, and fixes the port
number in test 2. The nondecimal test is removed, because it wants octal
and hexadecimal numbers in IPv4 addresses to be silently accepted by both
gethostbyname() and getaddrinfo(), which is what this patch forbids.
---
nss/digits_dots.c | 2 +-
nss/getaddrinfo.c | 2 +-
nss/tst-getaddrinfo2.c | 2 +-
nss/tst-getaddrinfo3.c | 9 ++-
resolv/Makefile | 1 -
resolv/tst-resolv-nondecimal.c | 139 ---------------------------------
6 files changed, 10 insertions(+), 145 deletions(-)
delete mode 100644 resolv/tst-resolv-nondecimal.c
@@ -158,7 +158,7 @@ __nss_hostname_digits_dots_context (struct resolv_context *ctx,
255.255.255.255? The test below will succeed
spuriously... ??? */
if (af == AF_INET)
- ok = __inet_aton_exact (name, (struct in_addr *) host_addr);
+ ok = inet_pton (AF_INET, name, (struct in_addr *) host_addr) > 0;
else
{
assert (af == AF_INET6);
@@ -879,7 +879,7 @@ text_to_binary_address (const char *name, const struct addrinfo *req,
assert (at != NULL);
memset (at->addr, 0, sizeof (at->addr));
- if (__inet_aton_exact (name, (struct in_addr *) at->addr) != 0)
+ if (inet_pton (AF_INET, name, (struct in_addr *) at->addr) == 1)
{
if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
at->family = AF_INET;
@@ -10,7 +10,7 @@
static int
do_test (void)
{
- const char portstr[] = "583";
+ const char portstr[] = "513";
int port = atoi (portstr);
struct addrinfo hints, *aires, *pai;
int rv;
@@ -34,8 +34,8 @@ do_test (void)
} \
else if (ai_res->ai_family != fam) \
{ \
- printf ("\
-getaddrinfo test %d return address of family %d, expected %d\n", \
+ printf ( \
+ "getaddrinfo test %d return address of family %d, expected %d\n", \
no, ai_res->ai_family, fam); \
result = 1; \
} \
@@ -144,6 +144,11 @@ getaddrinfo test %d return address of family %d, expected %d\n", \
hints.ai_socktype = SOCK_STREAM;
T (10, 0, "::ffff:127.0.0.1", AF_INET6, "::ffff:127.0.0.1");
+ memset (&hints, '\0', sizeof (hints));
+ hints.ai_family = AF_INET;
+ hints.ai_socktype = SOCK_STREAM;
+ T (11, EAI_NONAME, "1", AF_INET, "");
+
return result;
}
@@ -103,7 +103,6 @@ tests += \
tst-resolv-network \
tst-resolv-noaaaa \
tst-resolv-noaaaa-vc \
- tst-resolv-nondecimal \
tst-resolv-res_init-multi \
tst-resolv-search \
tst-resolv-semi-failure \
deleted file mode 100644
@@ -1,139 +0,0 @@
-/* Test name resolution behavior for octal, hexadecimal IPv4 addresses.
- Copyright (C) 2019-2024 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <https://www.gnu.org/licenses/>. */
-
-#include <netdb.h>
-#include <stdlib.h>
-#include <support/check.h>
-#include <support/check_nss.h>
-#include <support/resolv_test.h>
-#include <support/support.h>
-
-static void
-response (const struct resolv_response_context *ctx,
- struct resolv_response_builder *b,
- const char *qname, uint16_t qclass, uint16_t qtype)
-{
- /* The tests are not supposed send any DNS queries. */
- FAIL_EXIT1 ("unexpected DNS query for %s/%d/%d", qname, qclass, qtype);
-}
-
-static void
-run_query_addrinfo (const char *query, const char *address)
-{
- char *quoted_query = support_quote_string (query);
-
- struct addrinfo *ai;
- struct addrinfo hints =
- {
- .ai_socktype = SOCK_STREAM,
- .ai_protocol = IPPROTO_TCP,
- };
-
- char *context = xasprintf ("getaddrinfo \"%s\" AF_INET", quoted_query);
- char *expected = xasprintf ("address: STREAM/TCP %s 80\n", address);
- hints.ai_family = AF_INET;
- int ret = getaddrinfo (query, "80", &hints, &ai);
- check_addrinfo (context, ai, ret, expected);
- if (ret == 0)
- freeaddrinfo (ai);
- free (context);
-
- context = xasprintf ("getaddrinfo \"%s\" AF_UNSPEC", quoted_query);
- hints.ai_family = AF_UNSPEC;
- ret = getaddrinfo (query, "80", &hints, &ai);
- check_addrinfo (context, ai, ret, expected);
- if (ret == 0)
- freeaddrinfo (ai);
- free (expected);
- free (context);
-
- context = xasprintf ("getaddrinfo \"%s\" AF_INET6", quoted_query);
- expected = xasprintf ("flags: AI_V4MAPPED\n"
- "address: STREAM/TCP ::ffff:%s 80\n",
- address);
- hints.ai_family = AF_INET6;
- hints.ai_flags = AI_V4MAPPED;
- ret = getaddrinfo (query, "80", &hints, &ai);
- check_addrinfo (context, ai, ret, expected);
- if (ret == 0)
- freeaddrinfo (ai);
- free (expected);
- free (context);
-
- free (quoted_query);
-}
-
-static void
-run_query (const char *query, const char *address)
-{
- char *quoted_query = support_quote_string (query);
- char *context = xasprintf ("gethostbyname (\"%s\")", quoted_query);
- char *expected = xasprintf ("name: %s\n"
- "address: %s\n", query, address);
- check_hostent (context, gethostbyname (query), expected);
- free (context);
-
- context = xasprintf ("gethostbyname_r \"%s\"", quoted_query);
- struct hostent storage;
- char buf[4096];
- struct hostent *e = NULL;
- TEST_COMPARE (gethostbyname_r (query, &storage, buf, sizeof (buf),
- &e, &h_errno), 0);
- check_hostent (context, e, expected);
- free (context);
-
- context = xasprintf ("gethostbyname2 (\"%s\", AF_INET)", quoted_query);
- check_hostent (context, gethostbyname2 (query, AF_INET), expected);
- free (context);
-
- context = xasprintf ("gethostbyname2_r \"%s\" AF_INET", quoted_query);
- e = NULL;
- TEST_COMPARE (gethostbyname2_r (query, AF_INET, &storage, buf, sizeof (buf),
- &e, &h_errno), 0);
- check_hostent (context, e, expected);
- free (context);
- free (expected);
-
- free (quoted_query);
-
- /* The gethostbyname tests are always valid for getaddrinfo, but not
- vice versa. */
- run_query_addrinfo (query, address);
-}
-
-static int
-do_test (void)
-{
- struct resolv_test *aux = resolv_test_start
- ((struct resolv_redirect_config)
- {
- .response_callback = response,
- });
-
- run_query ("192.000.002.010", "192.0.2.8");
-
- /* Hexadecimal numbers are not accepted by gethostbyname. */
- run_query_addrinfo ("0xc0000210", "192.0.2.16");
- run_query_addrinfo ("192.0x234", "192.0.2.52");
-
- resolv_test_end (aux);
-
- return 0;
-}
-
-#include <support/test-driver.c>
--
2.45.2