Merge pull request #3815 from AndrzejKurek/cipher-optim-mem-fix
ssl_tls.c: Fix unchecked memory allocation
diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h
index 5534ec4..cf7f403 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -1912,7 +1912,11 @@
/**
* \def MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH
*
- * Enable modifying the maximum I/O buffer size.
+ * Enable modifying the maximum I/O buffer size in runtime.
+ *
+ * If the library runs out of memory during the resizing of an I/O buffer,
+ * there is no error returned. The operation continues as usual on an
+ * unchanged buffer without any negative impact on the flow.
*/
//#define MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 40b6f70..24dbb7d 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -666,6 +666,72 @@
return 0;
}
+
+#define BUFFER_UPSIZING 0
+#define BUFFER_DOWNSIZING 1
+static void handle_buffer_resizing( mbedtls_ssl_context *ssl, int downsizing,
+ uint32_t in_buf_new_len,
+ uint32_t out_buf_new_len )
+{
+ int modified = 0;
+ size_t written_in = 0, len_offset_in = 0;
+ size_t written_out = 0, iv_offset_out = 0, len_offset_out = 0;
+ if( ssl->in_buf != NULL )
+ {
+ written_in = ssl->in_msg - ssl->in_buf;
+ len_offset_in = ssl->in_len - ssl->in_buf;
+ if( ( downsizing && ssl->in_buf_len > in_buf_new_len && ssl->in_left < in_buf_new_len ) ||
+ ( !downsizing && ssl->in_buf_len < in_buf_new_len ) )
+ {
+ if( resize_buffer( &ssl->in_buf, in_buf_new_len, &ssl->in_buf_len ) != 0 )
+ {
+ /* No need to return an error here; The buffer will remain as
+ * is with no negative impact on the flow. */
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "input buffer resizing failed - out of memory" ) );
+ }
+ else
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating in_buf to %d", in_buf_new_len ) );
+ modified = 1;
+ }
+ }
+ }
+
+ if( ssl->out_buf != NULL )
+ {
+ written_out = ssl->out_msg - ssl->out_buf;
+ iv_offset_out = ssl->out_iv - ssl->out_buf;
+ len_offset_out = ssl->out_len - ssl->out_buf;
+ if( ( downsizing && ssl->out_buf_len > out_buf_new_len && ssl->out_left < out_buf_new_len ) ||
+ ( !downsizing && ssl->out_buf_len < out_buf_new_len ) )
+ {
+ if( resize_buffer( &ssl->out_buf, out_buf_new_len, &ssl->out_buf_len ) != 0 )
+ {
+ /* No need to return an error here; The buffer will remain as
+ * is with no negative impact on the flow. */
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "output buffer resizing failed - out of memory" ) );
+ }
+ else
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating out_buf to %d", out_buf_new_len ) );
+ modified = 1;
+ }
+ }
+ }
+ if( modified )
+ {
+ /* Update pointers here to avoid doing it twice. */
+ ssl_reset_in_out_pointers( ssl );
+ /* Fields below might not be properly updated with record
+ * splitting or with CID, so they are manually updated here. */
+ ssl->out_msg = ssl->out_buf + written_out;
+ ssl->out_len = ssl->out_buf + len_offset_out;
+ ssl->out_iv = ssl->out_buf + iv_offset_out;
+
+ ssl->in_msg = ssl->in_buf + written_in;
+ ssl->in_len = ssl->in_buf + len_offset_in;
+ }
+}
#endif /* MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH */
#if defined(MBEDTLS_SSL_HW_RECORD_ACCEL)
@@ -8819,62 +8885,8 @@
#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH)
/* If the buffers are too small - reallocate */
- {
- int modified = 0;
- size_t written_in = 0, len_offset_in = 0;
- size_t written_out = 0, iv_offset_out = 0, len_offset_out = 0;
- if( ssl->in_buf != NULL )
- {
- written_in = ssl->in_msg - ssl->in_buf;
- len_offset_in = ssl->in_len - ssl->in_buf;
- if( ssl->in_buf_len < MBEDTLS_SSL_IN_BUFFER_LEN )
- {
- if( resize_buffer( &ssl->in_buf, MBEDTLS_SSL_IN_BUFFER_LEN,
- &ssl->in_buf_len ) != 0 )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "input buffer resizing failed - out of memory" ) );
- }
- else
- {
- MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating in_buf to %d", MBEDTLS_SSL_IN_BUFFER_LEN ) );
- modified = 1;
- }
- }
- }
-
- if( ssl->out_buf != NULL )
- {
- written_out = ssl->out_msg - ssl->out_buf;
- iv_offset_out = ssl->out_iv - ssl->out_buf;
- len_offset_out = ssl->out_len - ssl->out_buf;
- if( ssl->out_buf_len < MBEDTLS_SSL_OUT_BUFFER_LEN )
- {
- if( resize_buffer( &ssl->out_buf, MBEDTLS_SSL_OUT_BUFFER_LEN,
- &ssl->out_buf_len ) != 0 )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "output buffer resizing failed - out of memory" ) );
- }
- else
- {
- MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating out_buf to %d", MBEDTLS_SSL_OUT_BUFFER_LEN ) );
- modified = 1;
- }
- }
- }
- if( modified )
- {
- /* Update pointers here to avoid doing it twice. */
- ssl_reset_in_out_pointers( ssl );
- /* Fields below might not be properly updated with record
- * splitting or with CID, so they are manually updated here. */
- ssl->out_msg = ssl->out_buf + written_out;
- ssl->out_len = ssl->out_buf + len_offset_out;
- ssl->out_iv = ssl->out_buf + iv_offset_out;
-
- ssl->in_msg = ssl->in_buf + written_in;
- ssl->in_len = ssl->in_buf + len_offset_in;
- }
- }
+ handle_buffer_resizing( ssl, BUFFER_UPSIZING, MBEDTLS_SSL_IN_BUFFER_LEN,
+ MBEDTLS_SSL_OUT_BUFFER_LEN );
#endif
/* All pointers should exist and can be directly freed without issue */
@@ -12065,64 +12077,9 @@
* processes datagrams and the fact that a datagram is allowed to have
* several records in it, it is possible that the I/O buffers are not
* empty at this stage */
- {
- int modified = 0;
- uint32_t buf_len = mbedtls_ssl_get_input_buflen( ssl );
- size_t written_in = 0, len_offset_in = 0;
- size_t written_out = 0, iv_offset_out = 0, len_offset_out = 0;
- if( ssl->in_buf != NULL )
- {
- written_in = ssl->in_msg - ssl->in_buf;
- len_offset_in = ssl->in_len - ssl->in_buf;
- if( ssl->in_buf_len > buf_len && ssl->in_left < buf_len )
- {
- if( resize_buffer( &ssl->in_buf, buf_len, &ssl->in_buf_len ) != 0 )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "input buffer resizing failed - out of memory" ) );
- }
- else
- {
- MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating in_buf to %d", buf_len ) );
- modified = 1;
- }
- }
- }
-
-
- buf_len = mbedtls_ssl_get_output_buflen( ssl );
- if( ssl->out_buf != NULL )
- {
- written_out = ssl->out_msg - ssl->out_buf;
- iv_offset_out = ssl->out_iv - ssl->out_buf;
- len_offset_out = ssl->out_len - ssl->out_buf;
- if( ssl->out_buf_len > mbedtls_ssl_get_output_buflen( ssl ) &&
- ssl->out_left < buf_len )
- {
- if( resize_buffer( &ssl->out_buf, buf_len, &ssl->out_buf_len ) != 0 )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "output buffer resizing failed - out of memory" ) );
- }
- else
- {
- MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating out_buf to %d", buf_len ) );
- modified = 1;
- }
- }
- }
- if( modified )
- {
- /* Update pointers here to avoid doing it twice. */
- ssl_reset_in_out_pointers( ssl );
- /* Fields below might not be properly updated with record
- * splitting or with CID, so they are manually updated here. */
- ssl->out_msg = ssl->out_buf + written_out;
- ssl->out_len = ssl->out_buf + len_offset_out;
- ssl->out_iv = ssl->out_buf + iv_offset_out;
-
- ssl->in_msg = ssl->in_buf + written_in;
- ssl->in_len = ssl->in_buf + len_offset_in;
- }
- }
+ handle_buffer_resizing( ssl, BUFFER_DOWNSIZING,
+ mbedtls_ssl_get_input_buflen( ssl ),
+ mbedtls_ssl_get_output_buflen( ssl ) );
#endif
}
diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c
index 1328ea2..ba4a3ac 100644
--- a/tinycrypt/ecc.c
+++ b/tinycrypt/ecc.c
@@ -1610,10 +1610,20 @@
static uECC_word_t regularize_k(const uECC_word_t * const k, uECC_word_t *k0,
uECC_word_t *k1)
{
+ wordcount_t num_n_words = NUM_ECC_WORDS;
bitcount_t num_n_bits = NUM_ECC_BITS;
+ /* With our constant NUM_ECC_BITS and NUM_ECC_WORDS the
+ * check (num_n_bits < ((bitcount_t)num_n_words * uECC_WORD_SIZE * 8) always would have "false" result (256 < 256),
+ * therefore Coverity warning may be detected. Removing of this line without changing the entire check will cause to
+ * array overrun.
+ * The entire check is not changed on purpose to be aligned with original tinycrypt
+ * implementation and to allow upstreaming to other curves if required.
+ * Coverity specific annotation may be added to silence warning if exists.
+ */
uECC_word_t carry = uECC_vli_add(k0, k, curve_n) ||
- uECC_vli_testBit(k0, num_n_bits);
+ (num_n_bits < ((bitcount_t)num_n_words * uECC_WORD_SIZE * 8) &&
+ uECC_vli_testBit(k0, num_n_bits));
uECC_vli_add(k1, k0, curve_n);