Skip to content

Commit

Permalink
[libc][obvious] fix printf failing to stop on %\0
Browse files Browse the repository at this point in the history
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
  • Loading branch information
michaelrj-google committed Nov 7, 2022
1 parent 583450f commit 28e312c
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 3 deletions.
11 changes: 9 additions & 2 deletions libc/src/stdio/printf_core/parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion libc/src/stdio/scanf_core/parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 13 additions & 0 deletions libc/test/src/stdio/printf_core/parser_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
13 changes: 13 additions & 0 deletions libc/test/src/stdio/scanf_core/parser_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down

0 comments on commit 28e312c

Please sign in to comment.