diff --git a/features/minimal-printf/TESTS/minimal-printf/compliance/main.cpp b/features/minimal-printf/TESTS/minimal-printf/compliance/main.cpp index 79e33589aa..c5aa85e93c 100755 --- a/features/minimal-printf/TESTS/minimal-printf/compliance/main.cpp +++ b/features/minimal-printf/TESTS/minimal-printf/compliance/main.cpp @@ -618,6 +618,7 @@ static control_t test_snprintf_x(const size_t call_count) return CaseNext; } +#if MBED_CONF_MINIMAL_PRINTF_ENABLE_FLOATING_POINT static control_t test_printf_f(const size_t call_count) { int result_baseline; @@ -672,6 +673,99 @@ static control_t test_snprintf_f(const size_t call_count) return CaseNext; } +#endif + + +/* Generic buffer overflow test function. + * Template parameters: + * 'T' is the type being tested + * 'buf_size' is the buffer size used in tests + * Function parameters: + * 'fmt' is the format to use for sprintf + * 'data' is the data that will be printed +*/ +template +static control_t test_snprintf_buffer_overflow_generic(const char *fmt, T data) +{ + char buffer_baseline[buf_size]; + char buffer_minimal[buf_size]; + int result_baseline; + int result_minimal; + + /* empty buffer test */ + result_minimal = mbed_snprintf(buffer_minimal, 0, fmt, data); + result_baseline = snprintf(buffer_baseline, 0, fmt, data); + TEST_ASSERT_EQUAL_INT(result_baseline, result_minimal); + + /* buffer isn't large enough, output needs to be truncated */ + result_minimal = mbed_snprintf(buffer_minimal, buf_size - 2, fmt, data); + result_baseline = snprintf(buffer_baseline, buf_size - 2, fmt, data); + TEST_ASSERT_EQUAL_STRING(buffer_baseline, buffer_minimal); + TEST_ASSERT_EQUAL_INT(result_baseline, result_minimal); + + /* buffer is one byte shorter than needed, string terminator must + be written and output must be truncated */ + result_minimal = mbed_snprintf(buffer_minimal, buf_size - 1, fmt, data); + result_baseline = snprintf(buffer_baseline, buf_size - 1, fmt, data); + TEST_ASSERT_EQUAL_STRING(buffer_baseline, buffer_minimal); + TEST_ASSERT_EQUAL_INT(result_baseline, result_minimal); + + /* buffer is just long enough */ + result_minimal = mbed_snprintf(buffer_minimal, buf_size, fmt, data); + result_baseline = snprintf(buffer_baseline, buf_size, fmt, data); + TEST_ASSERT_EQUAL_STRING(buffer_baseline, buffer_minimal); + TEST_ASSERT_EQUAL_INT(result_baseline, result_minimal); + + return CaseNext; +} + +/* Based on the generic buffer overflow function above, create tests for + each relevant data type. In each case, the buffer for printing will only + be large enough to fit the printed data. */ +static control_t test_snprintf_buffer_overflow_d(const size_t call_count) +{ + return test_snprintf_buffer_overflow_generic("d: %d", -1024); +} + +static control_t test_snprintf_buffer_overflow_ld(const size_t call_count) +{ + return test_snprintf_buffer_overflow_generic("ld: %ld", -1048576L); +} + +static control_t test_snprintf_buffer_overflow_lld(const size_t call_count) +{ + return test_snprintf_buffer_overflow_generic("lld: %lld", -1099511627776LL); +} + +static control_t test_snprintf_buffer_overflow_u(const size_t call_count) +{ + return test_snprintf_buffer_overflow_generic("u: %u", 1024); +} + +static control_t test_snprintf_buffer_overflow_lu(const size_t call_count) +{ + return test_snprintf_buffer_overflow_generic("lu: %lu", 1048576UL); +} + +static control_t test_snprintf_buffer_overflow_llu(const size_t call_count) +{ + return test_snprintf_buffer_overflow_generic("llu: %llu", 1099511627776ULL); +} + +static control_t test_snprintf_buffer_overflow_x(const size_t call_count) +{ + return test_snprintf_buffer_overflow_generic("x: 0x%x", 0x400); +} + +static control_t test_snprintf_buffer_overflow_lx(const size_t call_count) +{ + return test_snprintf_buffer_overflow_generic("lx: 0x%lx", 0x100000UL); +} + +static control_t test_snprintf_buffer_overflow_llx(const size_t call_count) +{ + return test_snprintf_buffer_overflow_generic("llx: 0x%llx", 0x10000000000ULL); +} utest::v1::status_t greentea_setup(const size_t number_of_cases) { @@ -690,6 +784,15 @@ Case cases[] = { Case("printf %f", test_printf_f), Case("snprintf %f", test_snprintf_f), #endif + Case("snprintf buffer overflow %d", test_snprintf_buffer_overflow_d), + Case("snprintf buffer overflow %ld", test_snprintf_buffer_overflow_ld), + Case("snprintf buffer overflow %lld", test_snprintf_buffer_overflow_lld), + Case("snprintf buffer overflow %u", test_snprintf_buffer_overflow_u), + Case("snprintf buffer overflow %lu", test_snprintf_buffer_overflow_lu), + Case("snprintf buffer overflow %llu", test_snprintf_buffer_overflow_llu), + Case("snprintf buffer overflow %x", test_snprintf_buffer_overflow_x), + Case("snprintf buffer overflow %lx", test_snprintf_buffer_overflow_lx), + Case("snprintf buffer overflow %llx", test_snprintf_buffer_overflow_llx), }; Specification specification(greentea_setup, cases, greentea_test_teardown_handler); diff --git a/features/minimal-printf/mbed_printf_implementation.c b/features/minimal-printf/mbed_printf_implementation.c index 404815fa0b..4c7f0c354d 100644 --- a/features/minimal-printf/mbed_printf_implementation.c +++ b/features/minimal-printf/mbed_printf_implementation.c @@ -133,6 +133,37 @@ static void mbed_minimal_formatted_string_void_pointer(char* buffer, size_t leng static void mbed_minimal_formatted_string_character(char* buffer, size_t length, int* result, char character); static void mbed_minimal_formatted_string_string(char* buffer, size_t length, int* result, const char* string); +/** + * @brief Print a single character, checking for buffer and size overflows. + * + * @param buffer The buffer to store output (NULL for stdout). + * @param[in] length The length of the buffer. + * @param result The current output location. + * @param[in] data The char to be printed. + */ +static void mbed_minimal_putchar(char *buffer, size_t length, int* result, char data) +{ + /* only continue if 'result' doesn't overflow */ + if ((*result >= 0) && (*result <= INT_MAX - 1)) + { + /* write data only if there's enough space */ + if ((size_t)*result < length) + { + if (buffer) + { + buffer[*result] = data; + } + else + { + MBED_PRINT_CHARACTER(data); + } + } + /* increment 'result' even if data was not written. This ensures that + 'mbed_minimal_formatted_string' returns the correct value. */ + *result += 1; + } +} + /** * @brief Print signed integer. * @@ -143,38 +174,24 @@ static void mbed_minimal_formatted_string_string(char* buffer, size_t length, in */ static void mbed_minimal_formatted_string_signed(char* buffer, size_t length, int* result, MBED_SIGNED_STORAGE value) { - /* only continue if buffer can fit at least 1 character and if - 'result' doesn't overflow */ - if ((*result >= 0) && (*result <= INT_MAX - 1) && ((size_t)*result + 1 <= length)) + MBED_UNSIGNED_STORAGE new_value = 0; + + /* if value is negative print sign and treat as positive number */ + if (value < 0) { - MBED_UNSIGNED_STORAGE new_value = 0; + /* write sign */ + mbed_minimal_putchar(buffer, length, result, '-'); - /* if value is negative print sign and treat as positive number */ - if (value < 0) - { - /* write sign */ - if (buffer) - { - buffer[*result] = '-'; - } - else - { - MBED_PRINT_CHARACTER('-'); - } - - *result += 1; - - /* get absolute value using two's complement */ - new_value = ~((MBED_UNSIGNED_STORAGE) value) + 1; - } - else - { - new_value = value; - } - - /* use unsigned long int function */ - mbed_minimal_formatted_string_unsigned(buffer, length, result, new_value); + /* get absolute value using two's complement */ + new_value = ~((MBED_UNSIGNED_STORAGE) value) + 1; } + else + { + new_value = value; + } + + /* use unsigned long int function */ + mbed_minimal_formatted_string_unsigned(buffer, length, result, new_value); } /** @@ -187,55 +204,32 @@ static void mbed_minimal_formatted_string_signed(char* buffer, size_t length, in */ static void mbed_minimal_formatted_string_unsigned(char* buffer, size_t length, int* result, MBED_UNSIGNED_STORAGE value) { - /* only continue if buffer can fit at least 1 character and if - 'result' doesn't overflow */ - if ((*result >= 0) && (*result <= INT_MAX - 1) && ((size_t)*result + 1 <= length)) + /* treat 0 as a corner case */ + if (value == 0) { - /* treat 0 as a corner case */ - if (value == 0) - { - if (buffer) - { - buffer[*result] = '0'; - } - else - { - MBED_PRINT_CHARACTER('0'); - } + mbed_minimal_putchar(buffer, length, result, '0'); + } + else + { + /* allocate 3 digits per byte */ + char scratch[sizeof(MBED_UNSIGNED_STORAGE) * 3] = { 0 }; - *result += 1; + size_t index = 0; + + /* write numbers in reverse order to scratch pad */ + for ( ; value > 0; index++) + { + /* use '0' as base and add digit */ + scratch[index] = '0' + (value % 10); + + /* shift value one decimal position */ + value = value / 10; } - else + + /* write scratch pad to buffer or output */ + for ( ; index > 0; index--) { - /* allocate 3 digits per byte */ - char scratch[sizeof(MBED_UNSIGNED_STORAGE) * 3] = { 0 }; - - size_t index = 0; - - /* write numbers in reverse order to scratch pad */ - for ( ; value > 0; index++) - { - /* use '0' as base and add digit */ - scratch[index] = '0' + (value % 10); - - /* shift value one decimal position */ - value = value / 10; - } - - /* write scratch pad to buffer or output */ - for ( ; (*result <= INT_MAX- 1) && ((size_t)*result < length) && (index > 0); index--) - { - if (buffer) - { - buffer[*result] = scratch[index - 1]; - } - else - { - MBED_PRINT_CHARACTER(scratch[index - 1]); - } - - *result += 1; - } + mbed_minimal_putchar(buffer, length, result, scratch[index - 1]); } } } @@ -252,9 +246,7 @@ static void mbed_minimal_formatted_string_hexadecimal(char* buffer, size_t lengt { bool print_leading_zero = false; - /* only continue each loop if buffer can fit at least 2 characters - and if 'result' doesn't overflow */ - for (int index = 7; (*result >= 0) && (*result <= INT_MAX - 2) && ((size_t)*result + 2 <= length) && (index >= 0); index--) + for (int index = 7; index >= 0; index--) { /* get most significant byte */ uint8_t output = value >> (8 * index); @@ -262,35 +254,19 @@ static void mbed_minimal_formatted_string_hexadecimal(char* buffer, size_t lengt /* only print leading zeros when set */ if (print_leading_zero || (output != 0) || (index == 0)) { - /* print zeroes after the first non-zero byte */ - print_leading_zero = true; - unsigned int nibble_one = (output >> 4); unsigned int nibble_two = (output & 0x0F); const char int2hex[16] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' }; - /* write to buffer or stdout */ - if (buffer) - { - if (nibble_one != 0) { - buffer[*result] = int2hex[nibble_one]; - *result += 1; - } - buffer[*result] = int2hex[nibble_two]; - *result += 1; - } - else - { - if (nibble_one != 0) { - MBED_PRINT_CHARACTER(int2hex[nibble_one]); - *result += 1; - } - MBED_PRINT_CHARACTER(int2hex[nibble_two]); - *result += 1; + if (print_leading_zero || nibble_one != 0) { + mbed_minimal_putchar(buffer, length, result, int2hex[nibble_one]); } + mbed_minimal_putchar(buffer, length, result, int2hex[nibble_two]); + /* print zeroes after the first non-zero byte */ + print_leading_zero = true; } } } @@ -305,28 +281,12 @@ static void mbed_minimal_formatted_string_hexadecimal(char* buffer, size_t lengt */ static void mbed_minimal_formatted_string_void_pointer(char* buffer, size_t length, int* result, const void* value) { - /* only continue if buffer can fit '0x' and twice the size of a void* - and if 'result' doesn't overflow */ - size_t needed = 2 + 2 * sizeof(void*); - if ((*result >= 0) && ((size_t)*result <= INT_MAX - needed) && ((size_t)*result + needed <= length)) - { - /* write leading 0x */ - if (buffer) - { - buffer[*result] = '0'; - buffer[*result + 1] = 'x'; - } - else - { - MBED_PRINT_CHARACTER('0'); - MBED_PRINT_CHARACTER('x'); - } + /* write leading 0x */ + mbed_minimal_putchar(buffer, length, result, '0'); + mbed_minimal_putchar(buffer, length, result, 'x'); - *result += 2; - - /* write rest as a regular hexadecimal number */ - mbed_minimal_formatted_string_hexadecimal(buffer, length, result, (ptrdiff_t) value); - } + /* write rest as a regular hexadecimal number */ + mbed_minimal_formatted_string_hexadecimal(buffer, length, result, (ptrdiff_t) value); } #if MBED_CONF_MINIMAL_PRINTF_ENABLE_FLOATING_POINT @@ -340,53 +300,48 @@ static void mbed_minimal_formatted_string_void_pointer(char* buffer, size_t leng */ static void mbed_minimal_formatted_string_double(char* buffer, size_t length, int* result, double value) { - /* only continue if buffer can fit at least 1 character and if - 'result' doesn't overflow */ - if ((*result >= 0) && (*result <= INT_MAX - 1) && ((size_t)*result + 1 <= length)) + /* get integer part */ + MBED_SIGNED_STORAGE integer = value; + + /* write integer part */ + mbed_minimal_formatted_string_signed(buffer, length, result, integer); + + /* write decimal point */ + mbed_minimal_formatted_string_character(buffer, length, result, '.'); + + /* get decimal part */ + double precision = 1.0; + + for (size_t index = 0; index < MBED_CONF_MINIMAL_PRINTF_SET_FLOATING_POINT_MAX_DECIMALS; index++) { - /* get integer part */ - MBED_SIGNED_STORAGE integer = value; - - /* write integer part */ - mbed_minimal_formatted_string_signed(buffer, length, result, integer); - - /* write decimal point */ - mbed_minimal_formatted_string_character(buffer, length, result, '.'); - - /* get decimal part */ - double precision = 1.0; - - for (size_t index = 0; index < MBED_CONF_MINIMAL_PRINTF_SET_FLOATING_POINT_MAX_DECIMALS; index++) - { - precision *= 10; - } - - value = (value - integer) * precision; - - /* convert to unsigned integer */ - MBED_UNSIGNED_STORAGE decimal = 0; - - if (value < 0) - { - MBED_SIGNED_STORAGE temp = value; - decimal = ~((MBED_UNSIGNED_STORAGE) temp) + 1; - } - else - { - decimal = value; - } - - /* round up or down */ - value -= decimal; - - if (!((value > -0.5) && (value < 0.5))) - { - decimal++; - } - - /* write decimal part */ - mbed_minimal_formatted_string_unsigned(buffer, length, result, decimal); + precision *= 10; } + + value = (value - integer) * precision; + + /* convert to unsigned integer */ + MBED_UNSIGNED_STORAGE decimal = 0; + + if (value < 0) + { + MBED_SIGNED_STORAGE temp = value; + decimal = ~((MBED_UNSIGNED_STORAGE) temp) + 1; + } + else + { + decimal = value; + } + + /* round up or down */ + value -= decimal; + + if (!((value > -0.5) && (value < 0.5))) + { + decimal++; + } + + /* write decimal part */ + mbed_minimal_formatted_string_unsigned(buffer, length, result, decimal); } #endif @@ -400,34 +355,24 @@ static void mbed_minimal_formatted_string_double(char* buffer, size_t length, in */ static void mbed_minimal_formatted_string_character(char* buffer, size_t length, int* result, char character) { - /* only continue if the buffer can fit 1 character and if - 'result' doesn't overflow */ - if ((*result >= 0) && (*result <= INT_MAX - 1) && ((size_t)*result + 1 <= length)) + /* write character */ + if (buffer) { - /* write character */ - if (buffer) - { - buffer[*result] = character; - } - else - { - /* convert \n to \r\n if enabled in platform configuration */ + mbed_minimal_putchar(buffer, length, result, character); + } + else + { + /* convert \n to \r\n if enabled in platform configuration */ #if MBED_CONF_PLATFORM_STDIO_CONVERT_NEWLINES - if (character == '\n' && mbed_stdio_out_prev != '\r') - { - MBED_PRINT_CHARACTER('\r'); - *result += 1; - } - - /* cache character */ - mbed_stdio_out_prev = character; -#endif - - /* write character to stdout */ - MBED_PRINT_CHARACTER(character); + if (character == '\n' && mbed_stdio_out_prev != '\r') + { + mbed_minimal_putchar(buffer, length, result, '\r'); } - *result += 1; + /* cache character */ + mbed_stdio_out_prev = character; +#endif + mbed_minimal_putchar(buffer, length, result, character); } } @@ -441,44 +386,10 @@ static void mbed_minimal_formatted_string_character(char* buffer, size_t length, */ static void mbed_minimal_formatted_string_string(char* buffer, size_t length, int* result, const char* string) { - /* only continue if the buffer can fit at least 1 character and if - 'result' doesn't overflow */ - if ((*result >= 0) && (*result <= INT_MAX - 1) && ((size_t)*result + 1 <= length)) + while (*string != '\0') { - /* count characters in string */ - size_t remaining = length - *result; - size_t string_length = 0; - - /* only count characters that will fit into buffer */ - while ((string[string_length] != '\0') && (string_length < remaining)) - { - string_length++; - } - - /* copy string to buffer */ - if (buffer) - { - /* ensure that the value of "result" doesn't overflow */ - if (string_length + *result > INT_MAX) - { - string_length = (size_t)INT_MAX - *result; - } - for (size_t index = 0; index < string_length; index++) - { - buffer[*result + index] = string[index]; - } - } - /* print string */ - else - { - for (size_t index = 0; index < string_length; index++) - { - MBED_PRINT_CHARACTER(string[index]); - } - } - - /* add length to counter */ - *result += string_length; + mbed_minimal_putchar(buffer, length, result, *string); + string ++; } } @@ -498,12 +409,23 @@ int mbed_minimal_formatted_string(char* buffer, size_t length, const char* forma MBED_INITIALIZE_PRINT(); int result = 0; + bool empty_buffer = false; /* ensure that function wasn't called with an empty buffer, or with or with a buffer size that is larger than the maximum 'int' value, or with a NULL format specifier */ - if (format && length > 0 && length <= INT_MAX) + if (format && length >= 0 && length <= INT_MAX) { + /* Make sure that there's always space for the NULL terminator */ + if (length > 0) + { + length --; + } + else + { + /* the buffer is empty, there's no place to write the terminator */ + empty_buffer = true; + } /* parse string */ for (size_t index = 0; format[index] != '\0'; index++) { @@ -765,10 +687,18 @@ int mbed_minimal_formatted_string(char* buffer, size_t length, const char* forma } } - /* if writing to buffer, NULL terminate string in reserved space*/ - if (buffer && ((size_t)result < length)) + if (buffer && !empty_buffer) { - buffer[result] = '\0'; + /* NULL-terminate the buffer no matter what. We use '<=' to compare instead of '<' + because we know that we initially reserved space for '\0' by decrementing length */ + if ((size_t)result <= length) + { + buffer[result] = '\0'; + } + else + { + buffer[length] = '\0'; + } } }