Don't use OPENSSL_strdup() for copying alpn_selected
authorMatt Caswell <matt@openssl.org>
Mon, 18 Jun 2018 10:30:21 +0000 (11:30 +0100)
committerMatt Caswell <matt@openssl.org>
Thu, 21 Jun 2018 10:07:45 +0000 (11:07 +0100)
An alpn_selected value containing NUL bytes in it will result in
ext.alpn_selected_len having a larger value than the number of bytes
allocated in ext.alpn_selected.

Issue found by OSS-fuzz.

Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6507)

ssl/ssl_asn1.c
ssl/ssl_sess.c

index 9af4b84..b56c5e9 100644 (file)
@@ -328,7 +328,8 @@ SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const unsigned char **pp,
 
     ret->ext.tick_lifetime_hint = (unsigned long)as->tlsext_tick_lifetime_hint;
     ret->ext.tick_age_add = as->tlsext_tick_age_add;
-    if (as->tlsext_tick) {
+    OPENSSL_free(ret->ext.tick);
+    if (as->tlsext_tick != NULL) {
         ret->ext.tick = as->tlsext_tick->data;
         ret->ext.ticklen = as->tlsext_tick->length;
         as->tlsext_tick->data = NULL;
@@ -355,11 +356,11 @@ SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const unsigned char **pp,
     ret->flags = (int32_t)as->flags;
     ret->ext.max_early_data = as->max_early_data;
 
+    OPENSSL_free(ret->ext.alpn_selected);
     if (as->alpn_selected != NULL) {
-        if (!ssl_session_strndup((char **)&ret->ext.alpn_selected,
-                                 as->alpn_selected))
-            goto err;
+        ret->ext.alpn_selected = as->alpn_selected->data;
         ret->ext.alpn_selected_len = as->alpn_selected->length;
+        as->alpn_selected->data = NULL;
     } else {
         ret->ext.alpn_selected = NULL;
         ret->ext.alpn_selected_len = 0;
@@ -367,6 +368,7 @@ SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const unsigned char **pp,
 
     ret->ext.max_fragment_len_mode = as->tlsext_max_fragment_len_mode;
 
+    OPENSSL_free(ret->ticket_appdata);
     if (as->ticket_appdata != NULL) {
         ret->ticket_appdata = as->ticket_appdata->data;
         ret->ticket_appdata_len = as->ticket_appdata->length;
index 0723765..fde4187 100644 (file)
@@ -220,13 +220,11 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket)
         dest->ext.ticklen = 0;
     }
 
-    if (src->ext.alpn_selected) {
-        dest->ext.alpn_selected =
-            (unsigned char*)OPENSSL_strndup((char*)src->ext.alpn_selected,
-                                            src->ext.alpn_selected_len);
-        if (dest->ext.alpn_selected == NULL) {
+    if (src->ext.alpn_selected != NULL) {
+        dest->ext.alpn_selected = OPENSSL_memdup(src->ext.alpn_selected,
+                                                 src->ext.alpn_selected_len);
+        if (dest->ext.alpn_selected == NULL)
             goto err;
-        }
     }
 
 #ifndef OPENSSL_NO_SRP