From 956c9e063db14fa3f52f8fee2fb5e597c43742c4 Mon Sep 17 00:00:00 2001
From: Paul Bakker <p.j.bakker@polarssl.org>
Date: Thu, 19 Dec 2013 14:42:28 +0100
Subject: [PATCH] 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)
---
 ChangeLog              |  4 ++++
 include/polarssl/ssl.h |  8 ++++----
 library/ssl_tls.c      | 15 +++++++++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7df5fa39a..3078c58e2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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
diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h
index 4b0c5f8bb..7e668f93b 100644
--- a/include/polarssl/ssl.h
+++ b/include/polarssl/ssl.h
@@ -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,9 +248,9 @@
 #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 */ 
+#define SSL_EMPTY_RENEGOTIATION_INFO    0xFF   /**< renegotiation info ext */
 
 /*
  * Supported Signature and Hash algorithms (For TLS 1.2)
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 75ba9075a..6ea282180 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -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 );