commit ee763495250b29fd32cb4026f17678ba30a59342
author Matt Caswell <matt@openssl.org> Tue Feb 20 10:20:20 2018 +0000
committer Matt Caswell <matt@openssl.org> Wed Feb 21 11:19:11 2018 +0000
tree 1e1b16cb83733d283fa1342eb63c7549a1266471
parent c7a47adca29e1ec761d639dd75336bb36ebf4a9b

Sanity check the ticket length before using key name/IV

This could in theory result in an overread - but due to the over allocation
of the underlying buffer does not represent a security issue.

Thanks to Fedor Indutny for reporting this issue.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/5414)

ssl/t1_lib.c

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 3965be9..57f9559 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1280,9 +1280,15 @@
     size_t mlen;
     unsigned char tick_hmac[EVP_MAX_MD_SIZE];
     HMAC_CTX *hctx = NULL;
-    EVP_CIPHER_CTX *ctx;
+    EVP_CIPHER_CTX *ctx = NULL;
     SSL_CTX *tctx = s->session_ctx;
 
+    /* Need at least keyname + iv */
+    if (eticklen < TLSEXT_KEYNAME_LENGTH + EVP_MAX_IV_LENGTH) {
+        ret = TICKET_NO_DECRYPT;
+        goto err;
+    }
+
     /* Initialize session ticket encryption and HMAC contexts */
     hctx = HMAC_CTX_new();
     if (hctx == NULL)
@@ -1294,8 +1300,9 @@
     }
     if (tctx->ext.ticket_key_cb) {
         unsigned char *nctick = (unsigned char *)etick;
-        int rv = tctx->ext.ticket_key_cb(s, nctick, nctick + 16,
-                                            ctx, hctx, 0);
+        int rv = tctx->ext.ticket_key_cb(s, nctick,
+                                         nctick + TLSEXT_KEYNAME_LENGTH,
+                                         ctx, hctx, 0);
         if (rv < 0)
             goto err;
         if (rv == 0) {
@@ -1307,7 +1314,7 @@
     } else {
         /* Check key name matches */
         if (memcmp(etick, tctx->ext.tick_key_name,
-                   sizeof(tctx->ext.tick_key_name)) != 0) {
+                   TLSEXT_KEYNAME_LENGTH) != 0) {
             ret = TICKET_NO_DECRYPT;
             goto err;
         }
@@ -1316,8 +1323,7 @@
                          EVP_sha256(), NULL) <= 0
             || EVP_DecryptInit_ex(ctx, EVP_aes_256_cbc(), NULL,
                                   tctx->ext.tick_aes_key,
-                                  etick
-                                  + sizeof(tctx->ext.tick_key_name)) <= 0) {
+                                  etick + TLSEXT_KEYNAME_LENGTH) <= 0) {
             goto err;
         }
     }