Reduced the input / output overhead with 200+ bytes and covered corner

case

The actual input / output buffer overhead is only 301 instead of 512.
This requires a proper check on the padding_idx to prevent out of bounds
reads.

Previously a remote party could potentially trigger an access error and
thus stop the application when sending a malicious packet having
MAX_CONTENT_LEN of data, 32 bytes of MAC and a decrypted padlen of .
This would result in reading from in_ctr + 13 + 32 + MAX_CONTENT_LEN - 1 - 1
for 256 bytes (including fake padding check). Or 13 + 32 bytes over the
buffer length.

We now reset padding_idx to 0, if it's clear that it will never be a
valid padding (padlen > msg_len || msg_len + padlen + 256 > buffer_len)
This commit is contained in:
Paul Bakker 2013-12-19 14:42:28 +01:00
parent f9c4953e39
commit 956c9e063d
3 changed files with 23 additions and 4 deletions

View File

@ -30,6 +30,10 @@ Bugfix
* Memory leak in benchmark application
* Fixed x509_crt_parse_path() bug on Windows platforms
Security
* Possible remotely-triggered out-of-bounds memory access fixed (found by
TrustInSoft)
= PolarSSL 1.3.2 released on 2013-11-04
Features
* PK tests added to test framework

View File

@ -237,8 +237,8 @@
#endif /* !POLARSSL_CONFIG_OPTIONS */
/*
* Allow an extra 512 bytes for the record header
* and encryption overhead (counter + MAC + padding)
* Allow an extra 301 bytes for the record header
* and encryption overhead: counter (8) + header (5) + MAC (32) + padding (256)
* and allow for a maximum of 1024 of compression expansion if
* enabled.
*/
@ -248,7 +248,7 @@
#define SSL_COMPRESSION_ADD 0
#endif
#define SSL_BUFFER_LEN (SSL_MAX_CONTENT_LEN + SSL_COMPRESSION_ADD + 512)
#define SSL_BUFFER_LEN (SSL_MAX_CONTENT_LEN + SSL_COMPRESSION_ADD + 301)
#define SSL_EMPTY_RENEGOTIATION_INFO 0xFF /**< renegotiation info ext */

View File

@ -1610,6 +1610,21 @@ static int ssl_decrypt_buf( ssl_context *ssl )
size_t pad_count = 0, real_count = 1;
size_t padding_idx = ssl->in_msglen - padlen - 1;
/*
* Padding is guaranteed to be incorrect if:
* 1. padlen - 1 > ssl->in_msglen
*
* 2. ssl->in_msglen + padlen >
* SSL_MAX_CONTENT_LEN + 256 (max padding)
*
* In both cases we reset padding_idx to a safe value (0) to
* prevent out-of-buffer reads.
*/
correct &= ( ssl->in_msglen >= padlen - 1 );
correct &= ( ssl->in_msglen + padlen <= SSL_MAX_CONTENT_LEN + 256 );
padding_idx *= correct;
for( i = 1; i <= 256; i++ )
{
real_count &= ( i <= padlen );