From patchwork Sat Aug 24 08:42:01 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?b?TWlrbMOzcyBNw6F0w6k=?= X-Patchwork-Id: 1976355 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=eHgRBf6I; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=libc-alpha-bounces~incoming=patchwork.ozlabs.org@sourceware.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WrVn703QRz1yXd for ; Sat, 24 Aug 2024 18:42:34 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 06D743858410 for ; Sat, 24 Aug 2024 08:42:32 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by sourceware.org (Postfix) with ESMTPS id 38F42385840B for ; Sat, 24 Aug 2024 08:42:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 38F42385840B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 38F42385840B Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::335 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1724488931; cv=none; b=l1bAWBfYARG76Y4rS9uY6ORVvmwojCSORrUaC8JiIIl3YDTDo+X+MDy+zfqDbI+QOxdjpTuKP+CT5Woye1DlNTlfZfkVhvrzfkXCHRjSLu6YG/3leNs9qlloSKTt8Qd67dI75vTHVsoQW5LCLpzWQyDRciK2G09wuZG28R2Fq9Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1724488931; c=relaxed/simple; bh=MRPUQ64qi/rhZGgFHwWIR3YzpMGi2BoFBqD/VBThajc=; h=DKIM-Signature:Message-ID:Date:MIME-Version:To:From:Subject; b=LZVuxvp8VJTVNjq2QHBLGqBFruHSKpthK/Yr8ya7Lsvl8irz0fpAUIqA3devvF1jAQWKMBOjFKYCY/W9anTRSFw5kC0md4MVzgRJonQkXiDICrT+/81aoFxBHYGpJOZb1YbmeFCtWVUY8iau1Sv7M0arI6sYCBezimqMs0ukg8M= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-x335.google.com with SMTP id 5b1f17b1804b1-429ec9f2155so22267205e9.2 for ; Sat, 24 Aug 2024 01:42:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724488926; x=1725093726; darn=sourceware.org; h=subject:from:content-language:to:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=MRPUQ64qi/rhZGgFHwWIR3YzpMGi2BoFBqD/VBThajc=; b=eHgRBf6IqfBne60eFQsxm4QBQBZaHn+MQtMgyRY5WeY2rvveilGo8/4Gqj98E6V2Zr 111vdN/VXibDP9t/pyvKAzvIDFfKfYtGiUxOF2F+Ze+NV2TvkuodD9x3R1icl6AmxW87 +erduRZ2sNwB3lbVeHyWAPxst9hiX6F6VaRTEjWJ20XYiU0EIDfp49k1yigjMXs2Thgm C/6HHwVpTucUO8WFCde2ruPdc1ATqg3hO3Yritr/MdeLT4Gg84j9DGn0OwuLrmmvWm5l kbAS1pZTxTIbDQyOzTBpXJE8wHD/E/Hj7DK9YTEGHMR21BSid5Dy9OlCHLDizsXNmicQ CzVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724488926; x=1725093726; h=subject:from:content-language:to:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=MRPUQ64qi/rhZGgFHwWIR3YzpMGi2BoFBqD/VBThajc=; b=f+rY4+jhCItp9YU0kmdYOxGTPH+ShnY7xB3Aa8Bt19V968MRoOXg+EjNLQYglZAlA6 84YUveL5zVGceKjzbh8ZVI/eh+T1lUuQUqnFCpgNmhfH3Ecm661kpVAtDN0pujo0Az+L mj+IoFTQMqEQhkgrFtnSuDPyq7XNhxyn7SZogDl+ugV5jUbMzSIUNTj8BvX4JjlCjvPO uGMVw4/1Sm5J3AFDsSBeLFI3b9OSRdXrIHpnhJc4Iwdj8NU0JoDbfzLNVeBVH4nplrbZ jiTkDfXEdjujs07IwTC8RmVXtkaLHuzXP/G94Cimg4atVnvRgg9Vslgge0nVPHL9E8MP NwKw== X-Gm-Message-State: AOJu0YzygiJAkGZio6t9SWjFl7A0XMEua1Gte1AeLGDSe3u7qf0/FRu6 Oo10zgU3FAQidkptFeC9WI4pkgy7rkv8uSFkus1Q9gk+oeD2gMd9mDVg2A== X-Google-Smtp-Source: AGHT+IFcCCO7APNRFD8MNS407E+zr4vxlfZtFWHk+mg5Z2Mk0u95Asktt8FTdvI8fjyJ1BDTxVmN8w== X-Received: by 2002:a5d:6702:0:b0:368:5a86:c1b7 with SMTP id ffacd0b85a97d-3731191b603mr3093949f8f.55.1724488925669; Sat, 24 Aug 2024 01:42:05 -0700 (PDT) Received: from [10.68.1.209] (84-236-126-109.pool.digikabel.hu. [84.236.126.109]) by smtp.googlemail.com with ESMTPSA id ffacd0b85a97d-37308161042sm6050306f8f.59.2024.08.24.01.42.04 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 24 Aug 2024 01:42:04 -0700 (PDT) Message-ID: <10d73c50-8462-47ba-ade8-167d56e9b4b8@gmail.com> Date: Sat, 24 Aug 2024 10:42:01 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: libc-alpha@sourceware.org Content-Language: en-US, hu-HU From: =?utf-8?b?TWlrbMOzcyBNw6F0w6k=?= Subject: [PATCH] [v2] nss: fix getaddrinfo() accepting garbage as valid IPv4 address X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, HK_RANDOM_ENVFROM, HK_RANDOM_FROM, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces~incoming=patchwork.ozlabs.org@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. From a81420d038647c17c3e20eaf2a8a1bca761b6755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20M=C3=A1t=C3=A9?= 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 diff --git a/nss/digits_dots.c b/nss/digits_dots.c index 15f8c383b8..cd4aad0b61 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_INET, 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; } diff --git a/resolv/Makefile b/resolv/Makefile index abff7fc007..56d495c25c 100644 --- a/resolv/Makefile +++ b/resolv/Makefile @@ -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 \ diff --git a/resolv/tst-resolv-nondecimal.c b/resolv/tst-resolv-nondecimal.c deleted file mode 100644 index 069b9252e2..0000000000 --- a/resolv/tst-resolv-nondecimal.c +++ /dev/null @@ -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 - . */ - -#include -#include -#include -#include -#include -#include - -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 -- 2.45.2