From patchwork Thu Oct 17 10:50:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antonio Quartulli X-Patchwork-Id: 1998499 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=unstable.cc header.i=a@unstable.cc header.a=rsa-sha256 header.s=20220809-q8oc header.b=mzHi+w5y; 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 4XTl4C1BHZz1xw2 for ; Thu, 17 Oct 2024 21:50:51 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 369F93858D37 for ; Thu, 17 Oct 2024 10:50:49 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from wilbur.contactoffice.com (wilbur.contactoffice.com [212.3.242.68]) by sourceware.org (Postfix) with ESMTPS id B590A3858D20 for ; Thu, 17 Oct 2024 10:50:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B590A3858D20 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=unstable.cc Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=unstable.cc ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B590A3858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=212.3.242.68 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729162234; cv=none; b=pqr+koLsmCbzWU2N9eLK+CKJFVGSQTkieXztgAcH4oE0zuj00EKODQ7a5kTCTmOCyy8i9PpJAxRt3vS5FCMkTWHVVCxlpp44sQyoYt1Xo/Zh9homHTTpPbtSSrQGL56ZFP7Esr/558yg4pM1/VDCCKdfl8PfsIn7OTAdPmo8JpI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729162234; c=relaxed/simple; bh=Y/j6cOzeV0kr6f8FieHx8xQffp5bkIZoTlztDFO8E8k=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=qeluai90AIf+KBHCvnUtgGmsQLW9avfNqsZXsOti/Ir9BxAgWI/ghbotEZdEiJ6iAeSI7dun98Xkm6zZTt3xFwiM5FRwahdpsK56rRj2ExuEzlBVZ3FCfQp7zMJnYc1jWlWglfa053WJyinhWgQnlyqAbuj6ghY0OfSJ+WVNFyk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from smtpauth2.co-bxl (smtpauth2.co-bxl [10.2.0.24]) by wilbur.contactoffice.com (Postfix) with ESMTP id CD627845; Thu, 17 Oct 2024 12:50:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1729162222; s=20220809-q8oc; d=unstable.cc; i=a@unstable.cc; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Transfer-Encoding; bh=nkulJVzowkO4abQOnd56PVlBk8WGUOSnNJwaBzBTFWo=; b=mzHi+w5ypJeJmsqih/NJpZ2cTSc8hQ7Cy/gY/tSTZhwex+i7trU+z+21Zmf2NPAl WVgXStOFc5i2EPuN9Gkhsn4je0BPmXj8pV8itGeFhkK7dx0zTM8t+BtOnXPMemjo5qQ /RGgR46oG2nQX67mX7k4g4r3+S1BlRSSMlY9+CwMnMCDfkLEKpKd41LclsA8AReXU0m B24cDm8DZwp9wD9g08H/skqQp0ZZGzPsDH9BA7ZX6I83QriNE/bvI+TdhstHXzTFnSr TpA7E1ARHUNb1qdF8ogiLmJjKhI0IN3RmBhAU5GlcgqroUb0cbEGjpLM/RAjNJwNbbC nxIbtViAvA== Received: by smtp.mailfence.com with ESMTPSA ; Thu, 17 Oct 2024 12:50:15 +0200 (CEST) From: Antonio Quartulli To: libc-alpha@sourceware.org Cc: Antonio Quartulli , Arne Schwabe , Cristina Nita-Rotaru , Anqi Chen Subject: [PATCH v4] nss: reject invalid port passed to getaddrinfo [BZ #16208] Date: Thu, 17 Oct 2024 12:50:08 +0200 Message-ID: <20241017105008.26036-1-a@unstable.cc> X-Mailer: git-send-email 2.45.2 MIME-Version: 1.0 X-ContactOffice-Account: com:375058688 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP 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 When passing a numeric string as port/service to getaddrinfo() representing a number larger than 65535, the function will not complain and will simply extract the lowest 16bits of the converted number. Specifically, the string is converted to int by calling strtoul() and later it is moved to gaih_servtuple.num after passing through htons(). For example, invoking getaddrinfo() with service equal to "70000" will result in no error and ai_addr->sin_port set to 4464. This issue was (re)discovered by researcher Anqi Chen while stress testing OpenVPN with invalid config parameters. Thanks a lot to Arne Schwabe for finding out that the root cause was hidden inside glibc. Similarly when passing an out of range value, no error is thrown while it should. Add proper checks to reject invalid values immediately. The 'num' member has been converted to unsigned long in order to properly store the result of strtoul(). Then, while checking for 'num' being greater than USHRT_MAX, we also catch overflows/out-of-range values (strtoul() returns ULONG_MAX in that case). Cc: Arne Schwabe Cc: Cristina Nita-Rotaru Reported-by: Anqi Chen Signed-off-by: Antonio Quartulli --- Changes from v3: * extended unit test with more notorius integer constants * rephrased comment * removed mention of 0 being invalid from commit message Changes from v2: * allowed specifying 0 as port number (equivalent to not passing any port) * changed unit-test to check success when passing 0 as port --- nss/Makefile | 1 + nss/getaddrinfo.c | 16 +++++++- nss/tst-getaddrinfo6.c | 84 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 nss/tst-getaddrinfo6.c diff --git a/nss/Makefile b/nss/Makefile index 9331b3308c..94acfd0e38 100644 --- a/nss/Makefile +++ b/nss/Makefile @@ -321,6 +321,7 @@ tests := \ tst-getaddrinfo \ tst-getaddrinfo2 \ tst-getaddrinfo3 \ + tst-getaddrinfo6 \ tst-gethnm \ tst-getpw \ tst-gshadow \ diff --git a/nss/getaddrinfo.c b/nss/getaddrinfo.c index 3ccd3905fa..26593dcd76 100644 --- a/nss/getaddrinfo.c +++ b/nss/getaddrinfo.c @@ -95,7 +95,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. struct gaih_service { const char *name; - int num; + unsigned long num; }; struct gaih_servtuple @@ -2375,7 +2375,19 @@ getaddrinfo (const char *name, const char *service, return EAI_NONAME; } - gaih_service.num = -1; + /* 0 means "no numeric port provided" (which is equivalent to + passing 0 as port) */ + gaih_service.num = 0; + } + /* port number must be in range [0, USHRT_MAX]. + Any other value is invalid. + strtoul returns ULONG_MAX in case of out-of-range + input and it is caught by this check */ + else if (gaih_service.num > USHRT_MAX) + { + /* the provided port number is invalid */ + __free_in6ai (in6ai); + return EAI_NONAME; } pservice = &gaih_service; diff --git a/nss/tst-getaddrinfo6.c b/nss/tst-getaddrinfo6.c new file mode 100644 index 0000000000..f2d0e34dca --- /dev/null +++ b/nss/tst-getaddrinfo6.c @@ -0,0 +1,84 @@ +/* Test invalid port/service lookup [BZ #16208]. + Copyright (C) 2014-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 int +do_one_test (const char *service, int expected_err) +{ + const int family[2] = { AF_INET, AF_INET6 }; + struct addrinfo hints, *aitop; + int index, gaierr; + int result = 0; + + for (index = 0; index < sizeof (family) / sizeof (family[0]); ++index) + { + memset (&hints, '\0', sizeof (hints)); + hints.ai_family = family[index]; + + gaierr = getaddrinfo (NULL, service, &hints, &aitop); + if (gaierr != expected_err) + { + printf ("FAIL getaddrinfo returned %d, should return %d\n", + gaierr, expected_err); + result = 1; + } + } + + return result; +} + + +static int +do_test (void) +{ + int err = 0; + + // 0 is accepted, test should succeed + err |= do_one_test ("0", 0); + err |= do_one_test ("70000", EAI_NONAME); + err |= do_one_test ("-1", EAI_NONAME); + + // INT_MIN + err |= do_one_test ("-2147483648", EAI_NONAME); + // INT_MAX + err |= do_one_test ("2147483647", EAI_NONAME); + // UINT_MAX / ULONG_MAX 32bit + err |= do_one_test ("4294967295", EAI_NONAME); + // ULONG_MAX 64bit / ULLONG_MAX + err |= do_one_test ("18446744073709551615", EAI_NONAME); + // LONG_MAX 64bit / LLONG_MAX + err |= do_one_test ("9223372036854775807", EAI_NONAME); + // LONG_MIN 64bit / LLONG_MIN + err |= do_one_test ("-9223372036854775808", EAI_NONAME); + + // ULONG_MAX 64bit +1 / ULLONG_MAX +1 + err |= do_one_test ("18446744073709551616", EAI_NONAME); + // LONG_MIN 64bit -1 / LLONG_MIN -1 + err |= do_one_test ("-9223372036854775809", EAI_NONAME); + + return err; +} +#define TEST_FUNCTION do_test () + +#include "../test-skeleton.c"