diff options
| -rw-r--r-- | ChangeLog | 14 | ||||
| -rw-r--r-- | debug/sprintf_chk.c | 4 | ||||
| -rw-r--r-- | debug/vsprintf_chk.c | 4 | ||||
| -rw-r--r-- | libio/Makefile | 7 | ||||
| -rw-r--r-- | libio/iovsprintf.c | 14 | ||||
| -rw-r--r-- | libio/libioP.h | 6 | ||||
| -rw-r--r-- | libio/tst-sprintf-chk-ub.c | 2 | ||||
| -rw-r--r-- | libio/tst-sprintf-ub.c | 102 |
8 files changed, 149 insertions, 4 deletions
diff --git a/ChangeLog b/ChangeLog index fda9b6209d..745042e4fd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2019-01-02 Gabriel F. T. Gomes <gabriel@inconstante.eti.br> + + * debug/sprintf_chk.c (___sprintf_chk): Use PRINTF_CHK. + * debug/vsprintf_chk.c (___vsprintf_chk): Likewise. + * libio/Makefile (tests): Add tst-sprintf-ub and + tst-sprintf-chk-ub. + (CFLAGS-tst-sprintf-ub.c): New variable. + (CFLAGS-tst-sprintf-chk-ub.c): Likewise. + * libio/iovsprintf.c (__vsprintf_internal): Only erase the + destination buffer and check for overflows in fortified mode. + * libio/libioP.h (PRINTF_CHK): New macro. + * libio/tst-sprintf-chk-ub.c: New file. + * libio/tst-sprintf-ub.c: Likewise. + 2019-01-02 Florian Weimer <fweimer@redhat.com> [BZ #24018] diff --git a/debug/sprintf_chk.c b/debug/sprintf_chk.c index 54d4a64255..7ef01653ee 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 7ef8cf38bc..c93ca4efb1 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 ee9ecc8f60..5bee83e55c 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 d3f23bec5c..90500380e2 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 fc13c8d624..8c75f15167 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..24cba39ec6 --- /dev/null +++ b/libio/tst-sprintf-ub.c @@ -0,0 +1,102 @@ +/* Test the sprintf (buf, "%s", buf) does not override buf. + Copyright (C) 2019 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> |