Merge remote-tracking branch 'origin/pr/2503' into mbedtls-2.16

* origin/pr/2503:
  Fix ChangeLog entry ordering
  Fix typo
  Add non-regression test for buffer overflow
  Improve documentation of mbedtls_mpi_write_string()
  Adapt ChangeLog
  Fix 1-byte buffer overflow in mbedtls_mpi_write_string()

ChangeLog

@@ -24,6 +24,8 @@ Bugfix
    * Fix potential memory leak in X.509 self test. Found and fixed by
      Junhwan Park, #2106.
    * Reduce stack usage of hkdf tests. Fixes #2195.
+   * Fix 1-byte buffer overflow in mbedtls_mpi_write_string() when
+     used with negative inputs. Found by Guido Vranken in #2404.
 
 Changes
    * Return from various debugging routines immediately if the

library/bignum.c

@@ -582,15 +582,20 @@ int mbedtls_mpi_write_string( const mbedtls_mpi *X, int radix,
     if( radix < 2 || radix > 16 )
         return( MBEDTLS_ERR_MPI_BAD_INPUT_DATA );
 
-    n = mbedtls_mpi_bitlen( X );
-    if( radix >=  4 ) n >>= 1;
-    if( radix >= 16 ) n >>= 1;
-    /*
-     * Round up the buffer length to an even value to ensure that there is
-     * enough room for hexadecimal values that can be represented in an odd
-     * number of digits.
-     */
-    n += 3 + ( ( n + 1 ) & 1 );
+    n = mbedtls_mpi_bitlen( X ); /* Number of bits necessary to present `n`. */
+    if( radix >=  4 ) n >>= 1;   /* Number of 4-adic digits necessary to present
+                                  * `n`. If radix > 4, this might be a strict
+                                  * overapproximation of the number of
+                                  * radix-adic digits needed to present `n`. */
+    if( radix >= 16 ) n >>= 1;   /* Number of hexadecimal digits necessary to
+                                  * present `n`. */
+
+    n += 1; /* Terminating null byte */
+    n += 1; /* Compensate for the divisions above, which round down `n`
+             * in case it's not even. */
+    n += 1; /* Potential '-'-sign. */
+    n += ( n & 1 ); /* Make n even to have enough space for hexadecimal writing,
+                     * which always uses an even number of hex-digits. */
 
     if( buflen < n )
     {
@@ -602,7 +607,10 @@ int mbedtls_mpi_write_string( const mbedtls_mpi *X, int radix,
     mbedtls_mpi_init( &T );
 
     if( X->s == -1 )
+    {
         *p++ = '-';
+        buflen--;
+    }
 
     if( radix == 16 )
     {

tests/suites/test_suite_mpi.data

@@ -25,6 +25,9 @@ mpi_read_write_string:16:"-20":10:"-32":100:0:0
 Base test mpi_read_write_string #3 (Negative decimal)
 mpi_read_write_string:16:"-23":16:"-23":100:0:0
 
+Base test mpi_read_write_string #4 (Buffer just fits)
+mpi_read_write_string:16:"-4":4:"-10":4:0:0
+
 Test mpi_read_write_string #1 (Invalid character)
 mpi_read_write_string:10:"a28":0:"":100:MBEDTLS_ERR_MPI_INVALID_CHARACTER:0
 

tests/suites/test_suite_mpi.function

@@ -294,6 +294,8 @@ void mpi_read_write_string( int radix_X, char * input_X, int radix_A,
 
     mbedtls_mpi_init( &X );
 
+    memset( str, '!', sizeof( str ) );
+
     TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == result_read );
     if( result_read == 0 )
     {
@@ -301,6 +303,7 @@ void mpi_read_write_string( int radix_X, char * input_X, int radix_A,
         if( result_write == 0 )
         {
             TEST_ASSERT( strcasecmp( str, input_A ) == 0 );
+            TEST_ASSERT( str[len] == '!' );
         }
     }