Merge pull request #10020 from bensze01/msvc-format-size-macros

Fix preprocessor guards for C99 format size specifiers
2025-03-14 16:20:51 +00:00 · 2025-03-13 10:09:06 +00:00 · 2025-03-13 10:09:06 +00:00 · 906d3cdff5
commit 906d3cdff5
parent 50432e42c8 24f11a366d
5 changed files with 109 additions and 9 deletions
--- a/ChangeLog.d/fix-msvc-version-guard-format-zu.txt
+++ b/ChangeLog.d/fix-msvc-version-guard-format-zu.txt
@ -0,0 +1,5 @@
+Bugfix
+   * Fix definition of MBEDTLS_PRINTF_SIZET to prevent runtime crashes that
+     occurred whenever SSL debugging was enabled on a copy of Mbed TLS built
+     with Visual Studio 2013 or MinGW.
+     Fixes #10017.
--- a/include/mbedtls/debug.h
+++ b/include/mbedtls/debug.h
@ -108,16 +108,16 @@
 *
 * This module provides debugging functions.
 */
-#if (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1800)
+#if defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER < 1900)
   #include <inttypes.h>
   #define MBEDTLS_PRINTF_SIZET     PRIuPTR
   #define MBEDTLS_PRINTF_LONGLONG  "I64d"
 #else \
-    /* (defined(__MINGW32__)  && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1800) */
+    /* defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER < 1900) */
   #define MBEDTLS_PRINTF_SIZET     "zu"
   #define MBEDTLS_PRINTF_LONGLONG  "lld"
 #endif \
-    /* (defined(__MINGW32__)  && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1800) */
+    /* defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER < 1900) */

 #if !defined(MBEDTLS_PRINTF_MS_TIME)
 #include <inttypes.h>
--- a/tests/suites/test_suite_debug.data
+++ b/tests/suites/test_suite_debug.data
@ -1,3 +1,12 @@
+printf "%" MBEDTLS_PRINTF_SIZET, 0
+printf_int_expr:PRINTF_SIZET:sizeof(size_t):0:"0"
+
+printf "%" MBEDTLS_PRINTF_LONGLONG, 0
+printf_int_expr:PRINTF_LONGLONG:sizeof(long long):0:"0"
+
+printf "%" MBEDTLS_PRINTF_MS_TIME, 0
+printf_int_expr:PRINTF_MS_TIME:sizeof(mbedtls_ms_time_t):0:"0"
+
 Debug print msg (threshold 1, level 0)
 debug_print_msg_threshold:1:0:"MyFile":999:"MyFile(0999)\: Text message, 2 == 2\n"

--- a/tests/suites/test_suite_debug.function
+++ b/tests/suites/test_suite_debug.function
@ -4,11 +4,34 @@
 #include "mbedtls/pk.h"
 #include <test/ssl_helpers.h>

+#if defined(_WIN32)
+#   include <stdlib.h>
+#   include <crtdbg.h>
+#endif
+
+// Dummy type for builds without MBEDTLS_HAVE_TIME
+#if !defined(MBEDTLS_HAVE_TIME)
+typedef int64_t mbedtls_ms_time_t;
+#endif
+
+typedef enum {
+    PRINTF_SIZET,
+    PRINTF_LONGLONG,
+    PRINTF_MS_TIME,
+} printf_format_indicator_t;
+
+const char *const printf_formats[] = {
+    [PRINTF_SIZET]    = "%" MBEDTLS_PRINTF_SIZET,
+    [PRINTF_LONGLONG] = "%" MBEDTLS_PRINTF_LONGLONG,
+    [PRINTF_MS_TIME]  = "%" MBEDTLS_PRINTF_MS_TIME,
+};
+
 struct buffer_data {
    char buf[2000];
    char *ptr;
 };

+#if defined(MBEDTLS_SSL_TLS_C)
 static void string_debug(void *data, int level, const char *file, int line, const char *str)
 {
    struct buffer_data *buffer = (struct buffer_data *) data;
@ -44,14 +67,77 @@ static void string_debug(void *data, int level, const char *file, int line, cons

    buffer->ptr = p;
 }
+#endif /* MBEDTLS_SSL_TLS_C */
+
+#if defined(_WIN32)
+static void noop_invalid_parameter_handler(
+    const wchar_t *expression,
+    const wchar_t *function,
+    const wchar_t *file,
+    unsigned int line,
+    uintptr_t pReserved)
+{
+    (void) expression;
+    (void) function;
+    (void) file;
+    (void) line;
+    (void) pReserved;
+}
+#endif /* _WIN32 */
+
 /* END_HEADER */

 /* BEGIN_DEPENDENCIES
- * depends_on:MBEDTLS_DEBUG_C:MBEDTLS_SSL_TLS_C
+ * depends_on:MBEDTLS_DEBUG_C
 * END_DEPENDENCIES
 */

 /* BEGIN_CASE */
+void printf_int_expr(int format_indicator, intmax_t sizeof_x, intmax_t x, char *result)
+{
+#if defined(_WIN32)
+    /* Windows treats any invalid format specifiers passsed to the CRT as fatal assertion failures.
+       Disable this behaviour temporarily, so the rest of the test cases can complete. */
+    _invalid_parameter_handler saved_handler =
+        _set_invalid_parameter_handler(noop_invalid_parameter_handler);
+
+    // Disable assertion pop-up window in Debug builds
+    int saved_report_mode = _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_REPORT_MODE);
+    _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG);
+#endif
+
+    const char *format = printf_formats[format_indicator];
+    char *output = NULL;
+    const size_t n = strlen(result);
+
+    /* Nominal case: buffer just large enough */
+    TEST_CALLOC(output, n + 1);
+    if ((size_t) sizeof_x <= sizeof(int)) { // Any smaller integers would be promoted to an int due to calling a vararg function
+        TEST_EQUAL(n, mbedtls_snprintf(output, n + 1, format, (int) x));
+    } else if (sizeof_x == sizeof(long)) {
+        TEST_EQUAL(n, mbedtls_snprintf(output, n + 1, format, (long) x));
+    } else if (sizeof_x == sizeof(long long)) {
+        TEST_EQUAL(n, mbedtls_snprintf(output, n + 1, format, (long long) x));
+    } else {
+        TEST_FAIL(
+            "sizeof_x <= sizeof(int) || sizeof_x == sizeof(long) || sizeof_x == sizeof(long long)");
+    }
+    TEST_MEMORY_COMPARE(result, n + 1, output, n + 1);
+
+exit:
+    mbedtls_free(output);
+    output = NULL;
+
+#if defined(_WIN32)
+    // Restore default Windows behaviour
+    _set_invalid_parameter_handler(saved_handler);
+    _CrtSetReportMode(_CRT_ASSERT, saved_report_mode);
+    (void) saved_report_mode;
+#endif
+}
+/* END_CASE */
+
+/* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C */
 void debug_print_msg_threshold(int threshold, int level, char *file,
                               int line, char *result_str)
 {
@ -89,7 +175,7 @@ exit:
 }
 /* END_CASE */

-/* BEGIN_CASE */
+/* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C */
 void mbedtls_debug_print_ret(char *file, int line, char *text, int value,
                             char *result_str)
 {
@ -124,7 +210,7 @@ exit:
 }
 /* END_CASE */

-/* BEGIN_CASE */
+/* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C */
 void mbedtls_debug_print_buf(char *file, int line, char *text,
                             data_t *data, char *result_str)
 {
@ -159,7 +245,7 @@ exit:
 }
 /* END_CASE */

-/* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C:!MBEDTLS_X509_REMOVE_INFO */
+/* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C:!MBEDTLS_X509_REMOVE_INFO */
 void mbedtls_debug_print_crt(char *crt_file, char *file, int line,
                             char *prefix, char *result_str)
 {
@ -199,7 +285,7 @@ exit:
 }
 /* END_CASE */

-/* BEGIN_CASE depends_on:MBEDTLS_BIGNUM_C */
+/* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C:MBEDTLS_BIGNUM_C */
 void mbedtls_debug_print_mpi(char *value, char *file, int line,
                             char *prefix, char *result_str)
 {
--- a/2
+++ b/2
@ -1 +1 @@
-Subproject commit 7d941e84a5b5c77f642186075ef45b3cc3214d57
+Subproject commit 399c5f9e1d71cb177eb0c16cb934755b409abe23