Fix potential use-after-free and memory leak
authorPauli <paul.dale@oracle.com>
Wed, 26 Jul 2017 00:04:05 +0000 (10:04 +1000)
committerPauli <paul.dale@oracle.com>
Wed, 26 Jul 2017 00:04:05 +0000 (10:04 +1000)
In function wait_for_async(), allocated async fds is freed if
`SSL_get_all_async_fds` fails, but later `fds` is used. Interestingly,
it is not freed when everything succeeds.

Rewrite the FD set loop to make it more readable and to not modify the allocated
pointer so it can be freed.

Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/3992)

apps/apps.c

index 4459be9..ad386a1 100644 (file)
@@ -2614,6 +2614,7 @@ void wait_for_async(SSL *s)
     fd_set asyncfds;
     OSSL_ASYNC_FD *fds;
     size_t numfds;
+    size_t i;
 
     if (!SSL_get_all_async_fds(s, NULL, &numfds))
         return;
@@ -2622,17 +2623,17 @@ void wait_for_async(SSL *s)
     fds = app_malloc(sizeof(OSSL_ASYNC_FD) * numfds, "allocate async fds");
     if (!SSL_get_all_async_fds(s, fds, &numfds)) {
         OPENSSL_free(fds);
+        return;
     }
 
     FD_ZERO(&asyncfds);
-    while (numfds > 0) {
-        if (width <= (int)*fds)
-            width = (int)*fds + 1;
-        openssl_fdset((int)*fds, &asyncfds);
-        numfds--;
-        fds++;
+    for (i = 0; i < numfds; i++) {
+        if (width <= (int)fds[i])
+            width = (int)fds[i] + 1;
+        openssl_fdset((int)fds[i], &asyncfds);
     }
     select(width, (void *)&asyncfds, NULL, NULL, NULL);
+    OPENSSL_free(fds);
 #endif
 }