Skip to content

Commit

Permalink
[libc][printf] Handle case where snprintf underflows for len = 0
Browse files Browse the repository at this point in the history
The code would write a null pointer always, even if len = 0, or if the
buffer pointer is null.

Test for this condition and add a unit test.

fixes #407
  • Loading branch information
travisg committed Jan 10, 2025
1 parent e7b7162 commit 6d9a0b5
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 4 deletions.
19 changes: 17 additions & 2 deletions app/tests/printf_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,31 @@ int printf_tests(int argc, const console_cmd_args *argv) {
/* make sure snprintf terminates at the right spot */
char buf[32];

memset(buf, 0, sizeof(buf));
memset(buf, 0x99, sizeof(buf));
err = sprintf(buf, "0123456789abcdef012345678");
printf("sprintf returns %d\n", err);
hexdump8(buf, sizeof(buf));

memset(buf, 0, sizeof(buf));
memset(buf, 0x99, sizeof(buf));
err = snprintf(buf, 15, "0123456789abcdef012345678");
printf("snprintf returns %d\n", err);
hexdump8(buf, sizeof(buf));

memset(buf, 0x99, sizeof(buf));
err = snprintf(buf, 1, "0123456789abcdef012345678");
printf("snprintf returns %d\n", err);
hexdump8(buf, sizeof(buf));

/* zero length is special case, should not write anything */
memset(buf, 0x99, sizeof(buf));
err = snprintf(buf, 0, "0123456789abcdef012345678");
printf("snprintf returns %d\n", err);
hexdump8(buf, sizeof(buf));

/* shold be able to pass null to the output buffer if zero length */
err = snprintf(NULL, 0, "0123456789abcdef012345678");
printf("snprintf returns %d\n", err);

return NO_ERROR;
}

Expand Down
9 changes: 7 additions & 2 deletions lib/libc/printf.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ static int _vsnprintf_output(const char *str, size_t len, void *state) {
count++;
}

// Return the count of the number of bytes that would be written even if the buffer
// wasn't large enough.
return count;
}

Expand All @@ -78,10 +80,13 @@ int vsnprintf(char *str, size_t len, const char *fmt, va_list ap) {
args.pos = 0;

wlen = _printf_engine(&_vsnprintf_output, (void *)&args, fmt, ap);
if (args.pos >= len)
if (len == 0) {
// do nothing, we can't null terminate the output
} else if (args.pos >= len) {
str[len-1] = '\0';
else
} else {
str[wlen] = '\0';
}
return wlen;
}

Expand Down
40 changes: 40 additions & 0 deletions lib/libc/test/printf_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,44 @@ bool snprintf_truncation_test() {
END_TEST;
}

// Test snprintf() with zero length.
bool snprintf_truncation_test_zero_length() {
BEGIN_TEST;

char buf[32];

memset(buf, 'x', sizeof(buf));
static const char str[26] = "0123456789abcdef012345678";

// Write with len = 0 a little ways into the buffer (to make sure it doesn't
// write to len -1).
int result = snprintf(buf + 4, 0, "%s", str);

// Check that snprintf() returns the length of the string that it would
// have written if the buffer was big enough.
EXPECT_EQ(result, (int)strlen(str));

// Check that snprintf() did not write anything.
for (auto c : buf)
EXPECT_EQ(c, 'x');

END_TEST;
}

// Test snprintf() with null pointer and zero length.
bool snprintf_truncation_test_null_buffer() {
BEGIN_TEST;

static const char str[26] = "0123456789abcdef012345678";
int result = snprintf(nullptr, 0, "%s", str);

// Check that snprintf() returns the length of the string that it would
// have written if the buffer was big enough.
EXPECT_EQ(result, (int)strlen(str));

END_TEST;
}

} // namespace

BEGIN_TEST_CASE(printf_tests)
Expand All @@ -467,4 +505,6 @@ RUN_TEST(alt_and_sign)
RUN_TEST(formatting)
//RUN_TEST(printf_field_width_and_precision_test)
RUN_TEST(snprintf_truncation_test)
RUN_TEST(snprintf_truncation_test_zero_length)
RUN_TEST(snprintf_truncation_test_null_buffer)
END_TEST_CASE(printf_tests)

0 comments on commit 6d9a0b5

Please sign in to comment.