Fortify vsnprintf in more cases.
Bug: http://b/30445072
Change-Id: I1893890f0e3b56533eef053eda1bd96a0b9a5119
diff --git a/libc/Android.bp b/libc/Android.bp
index 2ea6789..c882315 100644
--- a/libc/Android.bp
+++ b/libc/Android.bp
@@ -439,7 +439,6 @@
"upstream-openbsd/lib/libc/stdio/vfscanf.c",
"upstream-openbsd/lib/libc/stdio/vfwprintf.c",
"upstream-openbsd/lib/libc/stdio/vfwscanf.c",
- "upstream-openbsd/lib/libc/stdio/vsnprintf.c",
"upstream-openbsd/lib/libc/stdio/vsscanf.c",
"upstream-openbsd/lib/libc/stdio/vswprintf.c",
"upstream-openbsd/lib/libc/stdio/vswscanf.c",
diff --git a/libc/bionic/fortify.cpp b/libc/bionic/fortify.cpp
index 11abeb1..92db5d9 100644
--- a/libc/bionic/fortify.cpp
+++ b/libc/bionic/fortify.cpp
@@ -424,7 +424,12 @@
// Runtime implementation of __builtin____vsprintf_chk (used directly by compiler, not in headers).
extern "C" int __vsprintf_chk(char* dst, int /*flags*/,
size_t dst_len_from_compiler, const char* format, va_list va) {
- int result = vsnprintf(dst, dst_len_from_compiler, format, va);
+ // The compiler uses SIZE_MAX to mean "no idea", but our vsnprintf rejects sizes that large.
+ int result = vsnprintf(dst,
+ dst_len_from_compiler == SIZE_MAX ? SSIZE_MAX : dst_len_from_compiler,
+ format, va);
+
+ // Try to catch failures after the fact...
__check_buffer_access("vsprintf", "write into", result + 1, dst_len_from_compiler);
return result;
}
diff --git a/libc/stdio/stdio.cpp b/libc/stdio/stdio.cpp
index c673611..b709b40 100644
--- a/libc/stdio/stdio.cpp
+++ b/libc/stdio/stdio.cpp
@@ -46,6 +46,7 @@
#include "local.h"
#include "glue.h"
+#include "private/bionic_fortify.h"
#include "private/ErrnoRestorer.h"
#include "private/thread_private.h"
@@ -779,7 +780,7 @@
}
int sprintf(char* s, const char* fmt, ...) {
- PRINTF_IMPL(vsnprintf(s, INT_MAX, fmt, ap));
+ PRINTF_IMPL(vsprintf(s, fmt, ap));
}
int sscanf(const char* s, const char* fmt, ...) {
@@ -802,8 +803,34 @@
return vfscanf(stdin, fmt, ap);
}
+int vsnprintf(char* s, size_t n, const char* fmt, va_list ap) {
+ // stdio internals use int rather than size_t.
+ static_assert(INT_MAX <= SSIZE_MAX, "SSIZE_MAX too large to fit in int");
+
+ __check_count("vsnprintf", "size", n);
+
+ // Stdio internals do not deal correctly with zero length buffer.
+ char dummy;
+ if (n == 0) {
+ s = &dummy;
+ n = 1;
+ }
+
+ FILE f;
+ __sfileext fext;
+ _FILEEXT_SETUP(&f, &fext);
+ f._file = -1;
+ f._flags = __SWR | __SSTR;
+ f._bf._base = f._p = reinterpret_cast<unsigned char*>(s);
+ f._bf._size = f._w = n - 1;
+
+ int result = __vfprintf(&f, fmt, ap);
+ *f._p = '\0';
+ return result;
+}
+
int vsprintf(char* s, const char* fmt, va_list ap) {
- return vsnprintf(s, INT_MAX, fmt, ap);
+ return vsnprintf(s, SSIZE_MAX, fmt, ap);
}
int vwprintf(const wchar_t* fmt, va_list ap) {
diff --git a/libc/upstream-openbsd/lib/libc/stdio/vsnprintf.c b/libc/upstream-openbsd/lib/libc/stdio/vsnprintf.c
deleted file mode 100644
index 8b1a088..0000000
--- a/libc/upstream-openbsd/lib/libc/stdio/vsnprintf.c
+++ /dev/null
@@ -1,64 +0,0 @@
-/* $OpenBSD: vsnprintf.c,v 1.15 2009/11/09 00:18:28 kurt Exp $ */
-/*-
- * Copyright (c) 1990, 1993
- * The Regents of the University of California. All rights reserved.
- *
- * This code is derived from software contributed to Berkeley by
- * Chris Torek.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- * may be used to endorse or promote products derived from this software
- * without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <limits.h>
-#include <stdio.h>
-#include <string.h>
-#include "local.h"
-
-int
-vsnprintf(char *str, size_t n, const char *fmt, __va_list ap)
-{
- int ret;
- char dummy;
- FILE f;
- struct __sfileext fext;
-
- _FILEEXT_SETUP(&f, &fext);
-
- /* While snprintf(3) specifies size_t stdio uses an int internally */
- if (n > INT_MAX)
- n = INT_MAX;
- /* Stdio internals do not deal correctly with zero length buffer */
- if (n == 0) {
- str = &dummy;
- n = 1;
- }
- f._file = -1;
- f._flags = __SWR | __SSTR;
- f._bf._base = f._p = (unsigned char *)str;
- f._bf._size = f._w = n - 1;
- ret = __vfprintf(&f, fmt, ap);
- *f._p = '\0';
- return (ret);
-}
diff --git a/tests/stdio_test.cpp b/tests/stdio_test.cpp
index 8747dfc..ecba4ad 100644
--- a/tests/stdio_test.cpp
+++ b/tests/stdio_test.cpp
@@ -29,14 +29,20 @@
#include <vector>
+#include "BionicDeathTest.h"
#include "TemporaryFile.h"
#if defined(NOFORTIFY)
#define STDIO_TEST stdio_nofortify
+#define STDIO_DEATHTEST stdio_nofortify_DeathTest
#else
#define STDIO_TEST stdio
+#define STDIO_DEATHTEST stdio_DeathTest
#endif
+class stdio_DeathTest : public BionicDeathTest {};
+class stdio_nofortify_DeathTest : public BionicDeathTest {};
+
static void AssertFileIs(FILE* fp, const char* expected, bool is_fmemopen = false) {
rewind(fp);
@@ -1329,3 +1335,28 @@
ASSERT_EQ(-1, remove(td.dirname));
ASSERT_EQ(ENOENT, errno);
}
+
+TEST(STDIO_DEATHTEST, snprintf_30445072_known_buffer_size) {
+ char buf[16];
+ ASSERT_EXIT(snprintf(buf, atol("-1"), "hello"),
+ testing::KilledBySignal(SIGABRT),
+#if defined(NOFORTIFY)
+ "FORTIFY: vsnprintf: size .* > SSIZE_MAX"
+#else
+ "FORTIFY: vsnprintf: prevented .*-byte write into 16-byte buffer"
+#endif
+ );
+}
+
+TEST(STDIO_DEATHTEST, snprintf_30445072_unknown_buffer_size) {
+ std::string buf = "world";
+ ASSERT_EXIT(snprintf(&buf[0], atol("-1"), "hello"),
+ testing::KilledBySignal(SIGABRT),
+ "FORTIFY: vsnprintf: size .* > SSIZE_MAX");
+}
+
+TEST(STDIO_TEST, sprintf_30445072) {
+ std::string buf = "world";
+ sprintf(&buf[0], "hello");
+ ASSERT_EQ(buf, "hello");
+}
