mirror of
git://sourceware.org/git/glibc.git
synced 2025-03-06 20:58:33 +01:00
libio: asprintf should write NULL upon failure
This was suggested most recently by Solar Designer, noting that code replacing vsprintf with vasprintf in a security fix was subtly wrong: Re: GStreamer Security Advisory 2024-0003: Orc compiler stack-based buffer overflow <https://www.openwall.com/lists/oss-security/2024/07/26/2> Previous libc-alpha discussions: I: [PATCH] asprintf error handling fix <https://inbox.sourceware.org/libc-alpha/20011205185828.GA8376@ldv.office.alt-linux.org/> asprintf() issue <https://inbox.sourceware.org/libc-alpha/CANSoFxt-cdc-+C4u-rTENMtY4X9RpRSuv+axDswSPxbDgag8_Q@mail.gmail.com/> I don't think we need a compatibility symbol for this. As the GStreamer example shows, this change is much more likely to fix bugs than cause compatibility issues. Suggested-by: Dmitry V. Levin <ldv@altlinux.org> Suggested-by: Archie Cobbs <archie.cobbs@gmail.com> Suggested-by: Solar Designer <solar@openwall.com> Reviewed-by: Sam James <sam@gentoo.org>
This commit is contained in:
parent
7c22dcda27
commit
cb4692ce1e
4 changed files with 68 additions and 10 deletions
|
@ -88,6 +88,7 @@ tests = \
|
|||
test-fmemopen \
|
||||
test-fputs-unbuffered-full \
|
||||
test-fputws-unbuffered-full \
|
||||
tst-asprintf-null \
|
||||
tst-atime \
|
||||
tst-bz22415 \
|
||||
tst-bz24051 \
|
||||
|
|
51
libio/tst-asprintf-null.c
Normal file
51
libio/tst-asprintf-null.c
Normal file
|
@ -0,0 +1,51 @@
|
|||
/* Test that asprintf sets the buffer pointer to NULL on failure.
|
||||
Copyright (C) 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 <errno.h>
|
||||
#include <stdio.h>
|
||||
#include <support/check.h>
|
||||
#include <sys/resource.h>
|
||||
|
||||
static int
|
||||
do_test (void)
|
||||
{
|
||||
static const char sentinel[] = "sentinel";
|
||||
char *buf = (char *) sentinel;
|
||||
{
|
||||
/* Avoid -Wformat-overflow warning. */
|
||||
const char *volatile format = "%2000000000d %2000000000d";
|
||||
TEST_COMPARE (asprintf (&buf, format, 1, 2), -1);
|
||||
}
|
||||
if (errno != ENOMEM)
|
||||
TEST_COMPARE (errno, EOVERFLOW);
|
||||
TEST_VERIFY (buf == NULL);
|
||||
|
||||
/* Force ENOMEM in the test below. */
|
||||
struct rlimit rl;
|
||||
TEST_COMPARE (getrlimit (RLIMIT_AS, &rl), 0);
|
||||
rl.rlim_cur = 10 * 1024 * 1024;
|
||||
TEST_COMPARE (setrlimit (RLIMIT_AS, &rl), 0);
|
||||
|
||||
buf = (char *) sentinel;
|
||||
TEST_COMPARE (asprintf (&buf, "%20000000d", 1), -1);
|
||||
TEST_COMPARE (errno, ENOMEM);
|
||||
TEST_VERIFY (buf == NULL);
|
||||
return 0;
|
||||
}
|
||||
|
||||
#include <support/test-driver.c>
|
|
@ -92,7 +92,7 @@ __printf_buffer_flush_asprintf (struct __printf_buffer_asprintf *buf)
|
|||
|
||||
|
||||
int
|
||||
__vasprintf_internal (char **result_ptr, const char *format, va_list args,
|
||||
__vasprintf_internal (char **result, const char *format, va_list args,
|
||||
unsigned int mode_flags)
|
||||
{
|
||||
struct __printf_buffer_asprintf buf;
|
||||
|
@ -105,23 +105,23 @@ __vasprintf_internal (char **result_ptr, const char *format, va_list args,
|
|||
{
|
||||
if (buf.base.write_base != buf.direct)
|
||||
free (buf.base.write_base);
|
||||
*result = NULL;
|
||||
return done;
|
||||
}
|
||||
|
||||
/* Transfer to the final buffer. */
|
||||
char *result;
|
||||
size_t size = buf.base.write_ptr - buf.base.write_base;
|
||||
if (buf.base.write_base == buf.direct)
|
||||
{
|
||||
result = malloc (size + 1);
|
||||
if (result == NULL)
|
||||
*result = malloc (size + 1);
|
||||
if (*result == NULL)
|
||||
return -1;
|
||||
memcpy (result, buf.direct, size);
|
||||
memcpy (*result, buf.direct, size);
|
||||
}
|
||||
else
|
||||
{
|
||||
result = realloc (buf.base.write_base, size + 1);
|
||||
if (result == NULL)
|
||||
*result = realloc (buf.base.write_base, size + 1);
|
||||
if (*result == NULL)
|
||||
{
|
||||
free (buf.base.write_base);
|
||||
return -1;
|
||||
|
@ -129,8 +129,7 @@ __vasprintf_internal (char **result_ptr, const char *format, va_list args,
|
|||
}
|
||||
|
||||
/* Add NUL termination. */
|
||||
result[size] = '\0';
|
||||
*result_ptr = result;
|
||||
(*result)[size] = '\0';
|
||||
|
||||
return done;
|
||||
}
|
||||
|
|
|
@ -2578,7 +2578,14 @@ Allocation}) to hold the output, instead of putting the output in a
|
|||
buffer you allocate in advance. The @var{ptr} argument should be the
|
||||
address of a @code{char *} object, and a successful call to
|
||||
@code{asprintf} stores a pointer to the newly allocated string at that
|
||||
location.
|
||||
location. Current and future versions of @theglibc{} write a null
|
||||
pointer to @samp{*@var{ptr}} upon failure. To achieve similar
|
||||
behavior with previous versions, initialize @samp{*@var{ptr}} to a
|
||||
null pointer before calling @code{asprintf}. (Specifications for
|
||||
@code{asprintf} only require a valid pointer value in
|
||||
@samp{*@var{ptr}} if @code{asprintf} succeeds, but no implementations
|
||||
are known which overwrite a null pointer with a pointer that cannot be
|
||||
freed on failure.)
|
||||
|
||||
The return value is the number of characters allocated for the buffer, or
|
||||
less than zero if an error occurred. Usually this means that the buffer
|
||||
|
|
Loading…
Add table
Reference in a new issue