From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x29.google.com (mail-oa1-x29.google.com [IPv6:2001:4860:4864:20::29]) by sourceware.org (Postfix) with ESMTPS id E57503858D20 for ; Tue, 3 Oct 2023 18:07:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E57503858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oa1-x29.google.com with SMTP id 586e51a60fabf-1dce0f9e222so829004fac.0 for ; Tue, 03 Oct 2023 11:07:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1696356461; x=1696961261; darn=sourceware.org; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=XIanXUlB+iklERwkBLQpLK2c0jRGu7jz51YMF8UdiVM=; b=QKZECg3KSNnT7g3jBIQ7DzQ91QgzHxpSfkoJWTIO83McVRosKUNvcPuI1wHfvyVQPm oktG5eAoNQvbaMmh9oRbXohlZ9GBeHFfGqKONsDK4GrYS1Lod5qo7FEZc21mhKfznZUi pFZgRiW12xoZ3E3XXbkpIuzfsw3UknsdLq9dpXVmX8iM90w5EBQHAQc8jW6cQPn82DIK rsQtF8UBiuMZ/P9UP0YLxaK+WiOBaJRcG6/M2lvv8r/DHKparX2B2q9tdEcZqN9nkkaf tISVoDHlKoK0pRdNWZ6a5sC5N61RUG83/4hU6SXdBdX1Ni9idAGix82JLDWnx3X6Ecze mnKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696356461; x=1696961261; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=XIanXUlB+iklERwkBLQpLK2c0jRGu7jz51YMF8UdiVM=; b=KYmmx1tquq4xevIqWsI135vHwvK2R8P9kloFXmFZ00ZU6xpXFAk9Kl7BqpaWlFfNAI vJ3zmXiSahRAaZd/TeKzLbRBNpiShPy6wpaRl3UVsJjmbAtCC2JDHhMmYRAd/j4a0moh SP+Hv5N1ydPvHwi6WPkzOG1UOAojCQaZXN2lw1EktRKpoJFAfQGHDzO94xT8Zvm1I7iw TljnNgL7z+FQeqElfmaHfIzr6i0CXZke5M1nhyhblQZs5TnF7qE6CCyQd5FB2vPaquIC 5D1hd5JytCCcPRT2VDWkTG3H6YnC+WC0Vu9JVLlq3aNplHxGTuHuUszdXy8x8EcVqSLl 0xPA== X-Gm-Message-State: AOJu0YzA/Wh01Ebe2MAXyI3v42FYRXHL34aJMzegIOY7+uJQU0eLo899 wSe0B0qwOsquDN6srMkAigqJmw== X-Google-Smtp-Source: AGHT+IHODs4xKEV3tschvYXSrJB1yRX0E4/d4oL0P0tVz1WKbMjdsR6ggk7C5euY5rlsG/2IZlO9Xw== X-Received: by 2002:a05:6870:b156:b0:1d5:21f9:7272 with SMTP id a22-20020a056870b15600b001d521f97272mr329723oal.23.1696356461161; Tue, 03 Oct 2023 11:07:41 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c1:feaf:c9f1:61ab:649c:ad56? ([2804:1b3:a7c1:feaf:c9f1:61ab:649c:ad56]) by smtp.gmail.com with ESMTPSA id i67-20020a639d46000000b005782ad723casm1655192pgd.27.2023.10.03.11.07.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 03 Oct 2023 11:07:40 -0700 (PDT) Message-ID: <51a2d2d6-883b-44e5-b857-33337d3260e0@linaro.org> Date: Tue, 3 Oct 2023 15:07:38 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [committed 2/2] tunables: Terminate if end of input is reached (CVE-2023-4911) Content-Language: en-US To: Siddhesh Poyarekar , libc-alpha@sourceware.org Cc: Carlos O'Donell References: <20231003170811.64957-1-siddhesh@sourceware.org> <20231003170811.64957-3-siddhesh@sourceware.org> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <20231003170811.64957-3-siddhesh@sourceware.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 03/10/23 14:08, Siddhesh Poyarekar wrote: > The string parsing routine may end up writing beyond bounds of tunestr > if the input tunable string is malformed, of the form name=name=val. > This gets processed twice, first as name=name=val and next as name=val, > resulting in tunestr being name=name=val:name=val, thus overflowing > tunestr. > > Terminate the parsing loop at the first instance itself so that tunestr > does not overflow. > > This also fixes up tst-env-setuid-tunables to actually handle failures > correct and add new tests to validate the fix for this CVE. > > Signed-off-by: Siddhesh Poyarekar > Reviewed-by: Carlos O'Donell > --- > NEWS | 5 +++++ > elf/dl-tunables.c | 17 +++++++++------- > elf/tst-env-setuid-tunables.c | 37 +++++++++++++++++++++++++++-------- > 3 files changed, 44 insertions(+), 15 deletions(-) > > diff --git a/NEWS b/NEWS > index a94650da64..cc4b81f0ac 100644 > --- a/NEWS > +++ b/NEWS > @@ -64,6 +64,11 @@ Security related changes: > an application calls getaddrinfo for AF_INET6 with AI_CANONNAME, > AI_ALL and AI_V4MAPPED flags set. > > + CVE-2023-4911: If a tunable of the form NAME=NAME=VAL is passed in the > + environment of a setuid program and NAME is valid, it may result in a > + buffer overflow, which could be exploited to achieve escalated > + privileges. This flaw was introduced in glibc 2.34. > + > The following bugs are resolved with this release: > > [The release manager will add the list generated by > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 62b7332d95..cae67efa0a 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -180,11 +180,7 @@ parse_tunables (char *tunestr, char *valstring) > /* If we reach the end of the string before getting a valid name-value > pair, bail out. */ > if (p[len] == '\0') > - { > - if (__libc_enable_secure) > - tunestr[off] = '\0'; > - return; > - } > + break; > > /* We did not find a valid name-value pair before encountering the > colon. */ > @@ -244,9 +240,16 @@ parse_tunables (char *tunestr, char *valstring) > } > } > > - if (p[len] != '\0') > - p += len + 1; > + /* We reached the end while processing the tunable string. */ > + if (p[len] == '\0') > + break; > + > + p += len + 1; > } > + > + /* Terminate tunestr before we leave. */ > + if (__libc_enable_secure) > + tunestr[off] = '\0'; > } > So how should we handle what might be inconsistent invalid inputs like: glibc.malloc.mmap_threshold=4096=4096 Since glibc does not provide any TUNABLE_SECLEVEL_NONE, this tunables will be ignored. But for TUNABLE_SECLEVEL_NONE one, the value is still parsed by _dl_strtoul or stored in the tunable. > /* Enable the glibc.malloc.check tunable in SETUID/SETGID programs only when > diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c > index 7dfb0e073a..f0b92c97e7 100644 > --- a/elf/tst-env-setuid-tunables.c > +++ b/elf/tst-env-setuid-tunables.c > @@ -50,6 +50,8 @@ const char *teststrings[] = > "glibc.malloc.perturb=0x800:not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096", > "glibc.not_valid.check=2:glibc.malloc.mmap_threshold=4096", > "not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096", > + "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096", > + "glibc.malloc.check=2", > "glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096:glibc.malloc.check=2", > "glibc.malloc.check=4:glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096", > ":glibc.malloc.garbage=2:glibc.malloc.check=1", > @@ -68,6 +70,8 @@ const char *resultstrings[] = > "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096", > "glibc.malloc.mmap_threshold=4096", > "glibc.malloc.mmap_threshold=4096", > + "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096", > + "", > "", > "", > "", > @@ -81,11 +85,18 @@ test_child (int off) > { > const char *val = getenv ("GLIBC_TUNABLES"); > > + printf (" [%d] GLIBC_TUNABLES is %s\n", off, val); > + fflush (stdout); > if (val != NULL && strcmp (val, resultstrings[off]) == 0) > return 0; > > if (val != NULL) > - printf ("[%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val); > + printf (" [%d] Unexpected GLIBC_TUNABLES VALUE %s, expected %s\n", > + off, val, resultstrings[off]); > + else > + printf (" [%d] GLIBC_TUNABLES environment variable absent\n", off); > + > + fflush (stdout); > > return 1; > } > @@ -106,21 +117,26 @@ do_test (int argc, char **argv) > if (ret != 0) > exit (1); > > - exit (EXIT_SUCCESS); > + /* Special return code to make sure that the child executed all the way > + through. */ > + exit (42); > } > else > { > - int ret = 0; > - > /* Spawn tests. */ > for (int i = 0; i < array_length (teststrings); i++) > { > char buf[INT_BUFSIZE_BOUND (int)]; > > - printf ("Spawned test for %s (%d)\n", teststrings[i], i); > + printf ("[%d] Spawned test for %s\n", i, teststrings[i]); > snprintf (buf, sizeof (buf), "%d\n", i); > + fflush (stdout); > if (setenv ("GLIBC_TUNABLES", teststrings[i], 1) != 0) > - exit (1); > + { > + printf (" [%d] Failed to set GLIBC_TUNABLES: %m", i); > + support_record_failure (); > + continue; > + } > > int status = support_capture_subprogram_self_sgid (buf); > > @@ -128,9 +144,14 @@ do_test (int argc, char **argv) > if (WEXITSTATUS (status) == EXIT_UNSUPPORTED) > return EXIT_UNSUPPORTED; > > - ret |= status; > + if (WEXITSTATUS (status) != 42) > + { > + printf (" [%d] child failed with status %d\n", i, > + WEXITSTATUS (status)); > + support_record_failure (); > + } > } > - return ret; > + return 0; > } > } >