Set behavior of sprintf-like functions with overlapping source and destination

Message ID	20181220215409.8971-1-gabriel@inconstante.eti.br
State	New
Headers	show Return-Path: <libc-alpha-return-98675-incoming=patchwork.ozlabs.org@sourceware.org> DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-type; q=dns; s=default; b=tE9ho2qqD7o4vdDF0f7bYDP42GoZu xkJ8TD7JMfWUMBHe/FUqdKZKv3h4gpBKBRur0Mc9MmIeaqGD01J5HH7tHEAfNfe2 XN8KQEaurdxb44NwNmz816bfoA02ybRrl8nA7gDNXg5MVJbyTk/8/c48jIkN6ZBp VIoF51eQS1S1mo= Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk Sender: libc-alpha-owner@sourceware.org From: "Gabriel F. T. Gomes" <gabriel@inconstante.eti.br> To: <libc-alpha@sourceware.org> Subject: [PATCH] Set behavior of sprintf-like functions with overlapping source and destination Date: Thu, 20 Dec 2018 19:54:09 -0200 Message-ID: <20181220215409.8971-1-gabriel@inconstante.eti.br> MIME-Version: 1.0 Content-Type: text/plain
Series	Set behavior of sprintf-like functions with overlapping source and destination \| expand Set behavior of sprintf-like functions with overlapping source and destination

diff --git a/debug/sprintf_chk.c b/debug/sprintf_chk.c index 649e8ab4d5..dccdec92c3 100644 --- a/debug/sprintf_chk.c +++ b/debug/sprintf_chk.c @@ -29,6 +29,10 @@ ___sprintf_chk (char *s, int flag, size_t slen, const char *format, ...) va_list ap; int ret; + /* Regardless of the value of flag, let __vsprintf_internal know that + this is a call from *printf_chk. */ + mode |= PRINTF_CHK; + if (slen == 0) __chk_fail (); diff --git a/debug/vsprintf_chk.c b/debug/vsprintf_chk.c index c1b1a8da4f..3db6f624de 100644 --- a/debug/vsprintf_chk.c +++ b/debug/vsprintf_chk.c @@ -25,6 +25,10 @@ ___vsprintf_chk (char *s, int flag, size_t slen, const char *format, can only come from read-only format strings. */ unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0; + /* Regardless of the value of flag, let __vsprintf_internal know that + this is a call from *printf_chk. */ + mode |= PRINTF_CHK; + if (slen == 0) __chk_fail (); diff --git a/libio/Makefile b/libio/Makefile index 8239fba828..e1b0bf629d 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -64,7 +64,8 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \ bug-memstream1 bug-wmemstream1 \ tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \ tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \ - tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof + tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof \ + tst-sprintf-ub tst-sprintf-chk-ub tests-internal = tst-vtables tst-vtables-interposed tst-readline @@ -152,6 +153,10 @@ CFLAGS-oldtmpfile.c += -fexceptions CFLAGS-tst_putwc.c += -DOBJPFX=\"$(objpfx)\" +# These test cases intentionally use overlapping arguments +CFLAGS-tst-sprintf-ub.c += -Wno-restrict +CFLAGS-tst-sprintf-chk-ub.c += -Wno-restrict + tst_wprintf2-ARGS = "Some Text" test-fmemopen-ENV = MALLOC_TRACE=$(objpfx)test-fmemopen.mtrace diff --git a/libio/iovsprintf.c b/libio/iovsprintf.c index 08e4002625..f58519d172 100644 --- a/libio/iovsprintf.c +++ b/libio/iovsprintf.c @@ -77,8 +77,18 @@ __vsprintf_internal (char *string, size_t maxlen, sf._sbf._f._lock = NULL; #endif _IO_no_init (&sf._sbf._f, _IO_USER_LOCK, -1, NULL, NULL); - _IO_JUMPS (&sf._sbf) = &_IO_str_chk_jumps; - string[0] = '\0'; + /* When called from fortified sprintf/vsprintf, erase the destination + buffer and try to detect overflows. When called from regular + sprintf/vsprintf, do not erase the destination buffer, because + known user code relies on this behavior (even though its undefined + by ISO C), nor try to detect overflows. */ + if ((mode_flags & PRINTF_CHK) != 0) + { + _IO_JUMPS (&sf._sbf) = &_IO_str_chk_jumps; + string[0] = '\0'; + } + else + _IO_JUMPS (&sf._sbf) = &_IO_str_jumps; _IO_str_init_static_internal (&sf, string, (maxlen == -1) ? -1 : maxlen - 1, string); diff --git a/libio/libioP.h b/libio/libioP.h index 958ef9bffe..e5f8d2381d 100644 --- a/libio/libioP.h +++ b/libio/libioP.h @@ -705,9 +705,13 @@ extern int __vswprintf_internal (wchar_t *string, size_t maxlen, PRINTF_FORTIFY, when set to one, indicates that fortification checks are to be performed in input parameters. This is used by the __*printf_chk functions, which are used when _FORTIFY_SOURCE is - defined to 1 or 2. Otherwise, such checks are ignored. */ + defined to 1 or 2. Otherwise, such checks are ignored. + + PRINTF_CHK indicates, to the internal function being called, that the + call is originated from one of the __*printf_chk functions. */ #define PRINTF_LDBL_IS_DBL 0x0001 #define PRINTF_FORTIFY 0x0002 +#define PRINTF_CHK 0x0004 extern size_t _IO_getline (FILE *,char *, size_t, int, int); libc_hidden_proto (_IO_getline) diff --git a/libio/tst-sprintf-chk-ub.c b/libio/tst-sprintf-chk-ub.c new file mode 100644 index 0000000000..77ec64229a --- /dev/null +++ b/libio/tst-sprintf-chk-ub.c @@ -0,0 +1,2 @@ +#define _FORTIFY_SOURCE 2 +#include <tst-sprintf-ub.c> diff --git a/libio/tst-sprintf-ub.c b/libio/tst-sprintf-ub.c new file mode 100644 index 0000000000..35c6711122 --- /dev/null +++ b/libio/tst-sprintf-ub.c @@ -0,0 +1,102 @@ +/* Test the sprintf (buf, "%s", buf) does not override buf. + Copyright (C) 2018 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 + <http://www.gnu.org/licenses/>. */ + +#include <stdarg.h> +#include <stdio.h> +#include <string.h> + +#include <support/check.h> + +enum +{ + FUNCTION_FIRST, + FUNCTION_SPRINTF = FUNCTION_FIRST, + FUNCTION_SNPRINF, + FUNCTION_VSPRINTF, + FUNCTION_VSNPRINTF, + FUNCTION_LAST +}; + +static void +do_one_test (int function, char *buf, ...) +{ + va_list args; + char *arg; + + /* Expected results for fortified and non-fortified sprintf. */ +#if defined _FORTIFY_SOURCE && _FORTIFY_SOURCE > 1 + const char *expected = "CD"; +#else + const char *expected = "ABCD"; +#endif + + va_start (args, buf); + arg = va_arg (args, char *); + va_end (args); + + switch (function) + { + /* The regular sprintf and vsprintf functions do not override the + destination buffer, even if source and destination overlap. */ + case FUNCTION_SPRINTF: + sprintf (buf, "%sCD", buf); + TEST_COMPARE_STRING (buf, expected); + break; + case FUNCTION_VSPRINTF: + va_start (args, buf); + vsprintf (arg, "%sCD", args); + TEST_COMPARE_STRING (arg, expected); + va_end (args); + break; + /* On the other hand, snprint and vsnprint override overlapping + source and destination buffers. */ + case FUNCTION_SNPRINF: + snprintf (buf, 3, "%sCD", buf); + TEST_COMPARE_STRING (buf, "CD"); + break; + case FUNCTION_VSNPRINTF: + va_start (args, buf); + vsnprintf (arg, 3, "%sCD", args); + TEST_COMPARE_STRING (arg, "CD"); + va_end (args); + break; + default: + support_record_failure (); + } +} + +static int +do_test (void) +{ + char buf[8]; + int i; + + /* For each function in the enum do: + - reset the buffer to the initial state "AB"; + - call the function with buffer as source and destination; + - check for the desired behavior. */ + for (i = FUNCTION_FIRST; i < FUNCTION_LAST; i++) + { + strncpy (buf, "AB", 3); + do_one_test (i, buf, buf); + } + + return 0; +} + +#include <support/test-driver.c>

Set behavior of sprintf-like functions with overlapping source and destination

Commit Message

Comments

Patch