[libc-commits] [PATCH] D137367: [libc][obvious] fix printf failing to stop on %\0

[Michael Jones via Phabricator via libc-commits]

michaelrj created this revision.
michaelrj added a reviewer: sivachandra.
Herald added subscribers: libc-commits, ecnelises, tschuett.
Herald added projects: libc-project, All.
michaelrj requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137367

Files:
  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: libc/test/src/stdio/scanf_core/parser_test.cpp
===================================================================
--- libc/test/src/stdio/scanf_core/parser_test.cpp
+++ libc/test/src/stdio/scanf_core/parser_test.cpp
@@ -103,6 +103,19 @@
   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";
Index: libc/test/src/stdio/printf_core/parser_test.cpp
===================================================================
--- libc/test/src/stdio/printf_core/parser_test.cpp
+++ libc/test/src/stdio/printf_core/parser_test.cpp
@@ -102,6 +102,19 @@
   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: libc/src/stdio/scanf_core/parser.cpp
===================================================================
--- libc/src/stdio/scanf_core/parser.cpp
+++ libc/src/stdio/scanf_core/parser.cpp
@@ -74,7 +74,14 @@
       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: libc/src/stdio/printf_core/parser.cpp
===================================================================
--- libc/src/stdio/printf_core/parser.cpp
+++ libc/src/stdio/printf_core/parser.cpp
@@ -151,7 +151,11 @@
       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 @@
       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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137367.473055.patch
Type: text/x-patch
Size: 3180 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20221103/7059dc8c/attachment.bin>