Looking at the heartbeat code, I notice that neither of the process
heartbeat functions check whether RAND_pseudo_bytes returned success when
it is generating the heartbeat padding.

I don't know if there are real-world scenarios where this could happen, but
if so, I believe this would "bleed" 16 bytes of heap memory in the
heartbeat response.

A patch might look like this:

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index d8bcd58..a607d8c 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -1367,8 +1367,13 @@ dtls1_process_heartbeat(SSL *s)
                s2n(payload, bp);
                memcpy(bp, pl, payload);
                bp += payload;
+
                /* Random padding */
-               RAND_pseudo_bytes(bp, padding);
+               if (RAND_pseudo_bytes(bp, padding) < 0)
+                       {
+                       OPENSSL_free(buffer);
+                       return 0;
+                       }

                r = dtls1_write_bytes(s, TLS1_RT_HEARTBEAT, buffer,
write_length);

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index bcb99b8..f1a53e6 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -4000,8 +4000,13 @@ tls1_process_heartbeat(SSL *s)
                s2n(payload, bp);
                memcpy(bp, pl, payload);
                bp += payload;
+
                /* Random padding */
-               RAND_pseudo_bytes(bp, padding);
+               if (RAND_pseudo_bytes(bp, padding) < 0)
+                       {
+                       OPENSSL_free(buffer);
+                       return 0;
+                       }

                r = ssl3_write_bytes(s, TLS1_RT_HEARTBEAT, buffer, 3 +
payload + padding);

Comments?

Thanks,
Kylo

Reply via email to