Merge remote-tracking branch 'restricted/pr/535' into mbedtls-2.14-restricted

2 files changed, 46 insertions, 13 deletions

diff --git a/ChangeLog b/ChangeLog
index ab009d6c..683240ae 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,9 +5,17 @@ mbed TLS ChangeLog (Sorted per branch, date)
 Security
    * Fix timing variations and memory access variations in RSA PKCS#1 v1.5
      decryption that could lead to a Bleichenbacher-style padding oracle
-     attack. In TLS, this affects RSA-based ciphersuites without DHE or
-     ECDHE. Reported by Yuval Yarom, Eyal Ronen, Adi Shamir, David Wong and
-     Daniel Genkin.
+     attack. In TLS, this affects servers that accept ciphersuites based on
+     RSA decryption (i.e. ciphersuites whose name contains RSA but not
+     (EC)DH(E)). Reported by Eyal Ronen, Robert Gillham, Daniel Genkin, Adi
+     Shamir, David Wong and Yuval Yarom. CVE-2018-19608
+   * In mbedtls_mpi_write_binary(), don't leak the exact size of the number
+     via branching and memory access patterns. An attacker who could submit
+     a plaintext for RSA PKCS#1 v1.5 decryption but only observe the timing
+     of the decryption and not its result could nonetheless decrypt RSA
+     plaintexts and forge RSA signatures. Other asymmetric algorithms may
+     have been similarly vulnerable. Reported by Eyal Ronen, Robert Gillham,
+     Daniel Genkin, Adi Shamir, David Wong and Yuval Yarom.
 
 = mbed TLS 2.14.0 branch released 2018-11-19
 
diff --git a/library/bignum.c b/library/bignum.c
index ae5e7cfa..be4df2fe 100644
--- a/library/bignum.c
+++ b/library/bignum.c
@@ -321,6 +321,10 @@ int mbedtls_mpi_get_bit( const mbedtls_mpi *X, size_t pos )
     return( ( X->p[pos / biL] >> ( pos % biL ) ) & 0x01 );
 }
 
+/* Get a specific byte, without range checks. */
+#define GET_BYTE( X, i )                                \
+    ( ( ( X )->p[( i ) / ciL] >> ( ( ( i ) % ciL ) * 8 ) ) & 0xff )
+
 /*
  * Set a bit to a specific value of 0 or 1
  */
@@ -704,19 +708,40 @@ cleanup:
 /*
  * Export X into unsigned binary data, big endian
  */
-int mbedtls_mpi_write_binary( const mbedtls_mpi *X, unsigned char *buf, size_t buflen )
+int mbedtls_mpi_write_binary( const mbedtls_mpi *X,
+                              unsigned char *buf, size_t buflen )
 {
-    size_t i, j, n;
-
-    n = mbedtls_mpi_size( X );
-
-    if( buflen < n )
-        return( MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL );
+    size_t stored_bytes = X->n * ciL;
+    size_t bytes_to_copy;
+    unsigned char *p;
+    size_t i;
 
-    memset( buf, 0, buflen );
+    if( stored_bytes < buflen )
+    {
+        /* There is enough space in the output buffer. Write initial
+         * null bytes and record the position at which to start
+         * writing the significant bytes. In this case, the execution
+         * trace of this function does not depend on the value of the
+         * number. */
+        bytes_to_copy = stored_bytes;
+        p = buf + buflen - stored_bytes;
+        memset( buf, 0, buflen - stored_bytes );
+    }
+    else
+    {
+        /* The output buffer is smaller than the allocated size of X.
+         * However X may fit if its leading bytes are zero. */
+        bytes_to_copy = buflen;
+        p = buf;
+        for( i = bytes_to_copy; i < stored_bytes; i++ )
+        {
+            if( GET_BYTE( X, i ) != 0 )
+                return( MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL );
+        }
+    }
 
-    for( i = buflen - 1, j = 0; n > 0; i--, j++, n-- )
-        buf[i] = (unsigned char)( X->p[j / ciL] >> ((j % ciL) << 3) );
+    for( i = 0; i < bytes_to_copy; i++ )
+        p[bytes_to_copy - i - 1] = GET_BYTE( X, i );
 
     return( 0 );
 }