Merge "Fortify vsnprintf in more cases."

am: af211ab23f

Change-Id: Iccd96387222637d1a4c8eed4507ad8b5d21a16fb

libc/Android.bp

@@ -439,7 +439,6 @@ cc_library_static {
         "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",

libc/bionic/fortify.cpp

@@ -424,7 +424,12 @@ extern "C" int __snprintf_chk(char* dst, size_t supplied_size, int flags,
 // 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;
 }

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 snprintf(char* s, size_t n, const char* fmt, ...) {
 }
 
 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 @@ int vscanf(const char* fmt, va_list ap) {
   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) {

libc/upstream-openbsd/lib/libc/stdio/vsnprintf.c

@@ -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);
-}

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 @@ TEST(STDIO_TEST, remove) {
   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");
+}