Simplify mbedtls_debug_print_mpi and fix the case of empty bignums

Rewrite mbedtls_debug_print_mpi to be simpler and smaller. Leverage
mbedtls_mpi_bitlen() instead of manually looking for the leading
zeros.

Fix #4608: the old code made an invalid memory dereference when
X->n==0 (freshly initialized bignum with the value 0).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

ChangeLog.d/mbedtls_debug_print_mpi.txt

@@ -0,0 +1,5 @@
+Bugfix
+   * Fix a crash in mbedtls_mpi_debug_mpi on a bignum having 0 limbs. This
+     could notably be triggered by setting the TLS debug level to 3 or above
+     and using a Montgomery curve for the key exchange. Reported by lhuang04
+     in #4578. Fixes #4608.

library/debug.c

@@ -220,8 +220,8 @@ void mbedtls_debug_print_mpi( const mbedtls_ssl_context *ssl, int level,
                       const char *text, const mbedtls_mpi *X )
 {
     char str[DEBUG_BUF_SIZE];
-    int j, k, zeros = 1;
-    size_t i, n, idx = 0;
+    size_t bitlen;
+    size_t idx = 0;
 
     if( NULL == ssl              ||
         NULL == ssl->conf        ||
@@ -232,55 +232,43 @@ void mbedtls_debug_print_mpi( const mbedtls_ssl_context *ssl, int level,
         return;
     }
 
-    for( n = X->n - 1; n > 0; n-- )
-        if( X->p[n] != 0 )
-            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 ) );
+    bitlen = mbedtls_mpi_bitlen( X );
 
+    mbedtls_snprintf( str, sizeof( str ), "value of '%s' (%u bits) is:\n",
+                      text, (unsigned) bitlen );
     debug_send_line( ssl, level, file, line, str );
 
-    idx = 0;
-    for( i = n + 1, j = 0; i > 0; i-- )
+    if( bitlen == 0 )
     {
-        if( zeros && X->p[i - 1] == 0 )
-            continue;
-
-        for( k = sizeof( mbedtls_mpi_uint ) - 1; k >= 0; k-- )
+        str[0] = ' '; str[1] = '0'; str[2] = '0';
+        idx = 3;
+    }
+    else
+    {
+        int n;
+        for( n = ( bitlen - 1 ) / 8; n >= 0; n-- )
         {
-            if( zeros && ( ( X->p[i - 1] >> ( k << 3 ) ) & 0xFF ) == 0 )
-                continue;
-            else
-                zeros = 0;
-
-            if( j % 16 == 0 )
+            size_t limb_offset = n / sizeof( mbedtls_mpi_uint );
+            size_t offset_in_limb = n % sizeof( mbedtls_mpi_uint );
+            unsigned char octet =
+                ( X->p[limb_offset] >> ( offset_in_limb * 8 ) ) & 0xff;
+            mbedtls_snprintf( str + idx, sizeof( str ) - idx, " %02x", octet );
+            idx += 3;
+            /* Wrap lines after 16 octets that each take 3 columns */
+            if( idx >= 3 * 16 )
             {
-                if( j > 0 )
-                {
-                    mbedtls_snprintf( str + idx, sizeof( str ) - idx, "\n" );
-                    debug_send_line( ssl, level, file, line, str );
-                    idx = 0;
-                }
+                mbedtls_snprintf( str + idx, sizeof( str ) - idx, "\n" );
+                debug_send_line( ssl, level, file, line, str );
+                idx = 0;
             }
-
-            idx += mbedtls_snprintf( str + idx, sizeof( str ) - idx, " %02x", (unsigned int)
-                             ( X->p[i - 1] >> ( k << 3 ) ) & 0xFF );
-
-            j++;
         }
-
     }
 
-    if( zeros == 1 )
-        idx += mbedtls_snprintf( str + idx, sizeof( str ) - idx, " 00" );
-
-    mbedtls_snprintf( str + idx, sizeof( str ) - idx, "\n" );
-    debug_send_line( ssl, level, file, line, str );
+    if( idx != 0 )
+    {
+        mbedtls_snprintf( str + idx, sizeof( str ) - idx, "\n" );
+        debug_send_line( ssl, level, file, line, str );
+    }
 }
 #endif /* MBEDTLS_BIGNUM_C */