mbedtls_debug_print_mpi cause illegal access when MPI is empty

[wjzhang]

Bug

the mbedtls_debug_print_mpi will cause illegal access when MPI point is empty.
void mbedtls_debug_print_mpi( const mbedtls_ssl_context *ssl, int level,
const char *file, int line,
const char *text, const mbedtls_mpi *X )
{
char str[DEBUG_BUF_SIZE];
int j, k, zeros = 1;
size_t i, n, idx = 0;

if( ssl->conf == NULL || ssl->conf->f_dbg == NULL || X == NULL || level > debug_threshold )
    return;

for( n = X->n - 1; n > 0; n-- )
    if( X->p[n] != 0 )  **// when X->n == 0,  it's cause illegal access, because the n type is size_t, it unsigned.**
        break;

for( j = ( sizeof(mbedtls_mpi_uint) << 3 ) - 1; j >= 0; j-- )
    if( ( ( X->p[n] >> j ) & 1 ) != 0 )
        break;

mbedtls_snprintf( str + idx, sizeof( str ) - idx, "value of '%s' (%d bits) is:\n",
          text, (int) ( ( n * ( sizeof(mbedtls_mpi_uint) << 3 ) ) + j + 1 ) );

debug_send_line( ssl, level, file, line, str );

....

mbed TLS build:
Version: latest

[RonEld]

@wjzhang Thank you for your report!
We will look into this

[Patater]

Seems mbedtls_debug_print_mpi() needs to be robust enough not to crash when the bignum is empty.

[gvanem]

I confirm this bug when building libcurl with -DUSE_MBEDTLS and MBDEDTLS_DEBUG. The debug gets enabled by:

https://github.com/curl/curl/blob/master/lib/vtls/mbedtls.c#L515-L525:

#ifdef MBEDTLS_DEBUG
  /* In order to make that work in mbedtls MBEDTLS_DEBUG_C must be defined. */
  mbedtls_ssl_conf_dbg(&backend->config, mbed_debug, data);
  /* - 0 No debug
   * - 1 Error
   * - 2 State change
   * - 3 Informational
   * - 4 Verbose
   */
  mbedtls_debug_set_threshold(4);
#endif

Many (all?) curl -v https:// ... invocations generates this trace:

* <= read record
* dumping 'server key exchange' (296 bytes)
* 0110:  09 52 e7 a7 15 f6 58 7c 18 8d dd 32 61 d9 e2 dc  .R....X|...2a...
...

* value of 'ECDH: Qp(X)' (254 bits) is:
*  39 5b 2e 94 a5 b4 a3 67 eb b6 c5 92 54 de d3 88
*  b8 7f 06 27 6f 12 3d 77 cc d8 6b 8f 18 d0 77 a8

and Booms on the next debug-line I presume.

I simply patched it into this to fix it:

--- a/library/debug.c 2021-02-17 14:00:28
+++ b/library/debug.c 2021-02-17 15:25:07
@@ -231,6 +231,14 @@
         return;
     }

+   if (X->n <= 0)
+   {
+     debug_send_line( ssl, level, file, line, "X->n <= 0!!\n");
+     return;
+   }
+
     for( n = X->n - 1; n > 0; n-- )
         if( X->p[n] != 0 )
             break;

Doing a mbedtls_debug_set_threshold(2); in libcurl's vtls/mbedtls.c also fixes it (but hides the bug).

[tom-cosgrove-arm]

This was fixed as part of #4604, so closing