diff mbox series

nss: fix getaddrinfo() accepting garbage as valid IPv4 address

Message ID e2b4cf98-b869-4c8f-92fb-1544a5915121@gmail.com
State New
Headers show
Series nss: fix getaddrinfo() accepting garbage as valid IPv4 address | expand

Commit Message

Miklós Máté Aug. 17, 2024, 8:13 a.m. UTC 
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).

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.

Comments

Miklós Máté Aug. 17, 2024, 11:45 a.m. UTC | #1 
On 17/08/2024 10:13, Miklós Máté wrote:
> <snip>

Now I see that in digits_dots.c I made a typo, it should be AF_INET, 
this is why nss/tst-gethnm fails. Should I resubmit?
Adhemerval Zanella Aug. 19, 2024, 1:09 p.m. UTC | #2 
On 17/08/24 08:45, Miklós Máté wrote:
> On 17/08/2024 10:13, Miklós Máté wrote:
>> <snip>
> 
> Now I see that in digits_dots.c I made a typo, it should be AF_INET, this is why nss/tst-gethnm fails. Should I resubmit?
>

Please do, also check if this was the case of aarch64/arm CI failures [1].

[1] https://patchwork.sourceware.org/project/glibc/patch/e2b4cf98-b869-4c8f-92fb-1544a5915121@gmail.com/
diff mbox series

Patch

From c4ba83107fea1d61a309bd30370a357a5a4c053d 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] nss: fix getaddrinfo() accepting garbage as valid IPv4
 address

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).

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.
---
 nss/digits_dots.c      | 2 +-
 nss/getaddrinfo.c      | 2 +-
 nss/tst-getaddrinfo2.c | 2 +-
 nss/tst-getaddrinfo3.c | 9 +++++++--
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/nss/digits_dots.c b/nss/digits_dots.c
index 15f8c383b8..5d6cc4b1a2 100644
--- a/nss/digits_dots.c
+++ b/nss/digits_dots.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_INET6, name, (struct in_addr *) host_addr) > 0;
 		  else
 		    {
 		      assert (af == AF_INET6);
diff --git a/nss/getaddrinfo.c b/nss/getaddrinfo.c
index 3ccd3905fa..6e6741e9de 100644
--- a/nss/getaddrinfo.c
+++ b/nss/getaddrinfo.c
@@ -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;
diff --git a/nss/tst-getaddrinfo2.c b/nss/tst-getaddrinfo2.c
index d8be4a8e8f..7a884a9e3f 100644
--- a/nss/tst-getaddrinfo2.c
+++ b/nss/tst-getaddrinfo2.c
@@ -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;
diff --git a/nss/tst-getaddrinfo3.c b/nss/tst-getaddrinfo3.c
index 5077f311fc..255ac1ef74 100644
--- a/nss/tst-getaddrinfo3.c
+++ b/nss/tst-getaddrinfo3.c
@@ -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;
 }
 
-- 
2.45.2