aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--ChangeLog14
-rw-r--r--debug/sprintf_chk.c4
-rw-r--r--debug/vsprintf_chk.c4
-rw-r--r--libio/Makefile7
-rw-r--r--libio/iovsprintf.c14
-rw-r--r--libio/libioP.h6
-rw-r--r--libio/tst-sprintf-chk-ub.c2
-rw-r--r--libio/tst-sprintf-ub.c102
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>