[libc][obvious] fix printf failing to stop on %\0
authorMichael Jones <michaelrj@google.com>
Thu, 3 Nov 2022 22:21:30 +0000 (15:21 -0700)
committerMichael Jones <michaelrj@google.com>
Mon, 7 Nov 2022 18:44:30 +0000 (10:44 -0800)
Previously, the printf parser would treat "%\0" as a conversion with the
name "\0", and advance past the null byte causing a buffer overflow.
This patch corrects that in both printf and scanf.

Reviewed By: sivachandra

Differential Revision: https://reviews.llvm.org/D137367

libc/src/stdio/printf_core/parser.cpp
libc/src/stdio/scanf_core/parser.cpp
libc/test/src/stdio/printf_core/parser_test.cpp
libc/test/src/stdio/scanf_core/parser_test.cpp

index 38d2e1e..6a27c1c 100644 (file)
@@ -151,7 +151,11 @@ FormatSection Parser::get_next_section() {
       section.has_conv = false;
       break;
     }
-    ++cur_pos;
+    // If the end of the format section is on the '\0'. This means we need to
+    // not advance the cur_pos.
+    if (str[cur_pos] != '\0')
+      ++cur_pos;
+
   } else {
     // raw section
     section.has_conv = false;
@@ -372,7 +376,10 @@ Parser::TypeDesc Parser::get_type_desc(size_t index) {
       if (conv_index == index)
         return conv_size;
     }
-    ++local_pos;
+    // If the end of the format section is on the '\0'. This means we need to
+    // not advance the local_pos.
+    if (str[local_pos] != '\0')
+      ++local_pos;
   }
 
   // If there is no size for the requested index, then just guess that it's an
index 31dd118..76e658e 100644 (file)
@@ -74,7 +74,14 @@ FormatSection Parser::get_next_section() {
       section.output_ptr = GET_ARG_VAL_SIMPLEST(void *, conv_index);
     }
 
-    ++cur_pos;
+    // If the end of the format section is on the '\0'. This means we need to
+    // not advance the cur_pos and we should not count this has having a
+    // conversion.
+    if (str[cur_pos] != '\0') {
+      ++cur_pos;
+    } else {
+      section.has_conv = false;
+    }
 
     // If the format is a bracketed one, then we need to parse out the insides
     // of the brackets.
index 0684ebc..3ae8bf4 100644 (file)
@@ -102,6 +102,19 @@ TEST(LlvmLibcPrintfParserTest, EvalOneArg) {
   ASSERT_PFORMAT_EQ(expected, format_arr[0]);
 }
 
+TEST(LlvmLibcPrintfParserTest, EvalBadArg) {
+  __llvm_libc::printf_core::FormatSection format_arr[10];
+  const char *str = "%\0abc";
+  int arg1 = 12345;
+  evaluate(format_arr, str, arg1);
+
+  __llvm_libc::printf_core::FormatSection expected;
+  expected.has_conv = false;
+  expected.raw_string = {str, 1};
+
+  ASSERT_PFORMAT_EQ(expected, format_arr[0]);
+}
+
 TEST(LlvmLibcPrintfParserTest, EvalOneArgWithFlags) {
   __llvm_libc::printf_core::FormatSection format_arr[10];
   const char *str = "%+-0 #d";
index e2ed4b0..3d2c081 100644 (file)
@@ -103,6 +103,19 @@ TEST(LlvmLibcScanfParserTest, EvalOneArg) {
   ASSERT_SFORMAT_EQ(expected, format_arr[0]);
 }
 
+TEST(LlvmLibcScanfParserTest, EvalBadArg) {
+  __llvm_libc::scanf_core::FormatSection format_arr[10];
+  const char *str = "%\0abc";
+  int arg1 = 12345;
+  evaluate(format_arr, str, &arg1);
+
+  __llvm_libc::scanf_core::FormatSection expected;
+  expected.has_conv = false;
+  expected.raw_string = {str, 1};
+
+  ASSERT_SFORMAT_EQ(expected, format_arr[0]);
+}
+
 TEST(LlvmLibcScanfParserTest, EvalOneArgWithFlag) {
   __llvm_libc::scanf_core::FormatSection format_arr[10];
   const char *str = "%*d";