Dear OpenSSL developers,
when having a glance a the source code of the heartbeat feature, I
spontaneously found a flaw in then programming style that probably
concealed what caused the vulnerability.
I created a pull request on Github. Then I noticed that someone
else wrote a similar fix. I submitted a pull request to this user,
"s0enke", and he merged. You can find s0enke's and my porposal as
pull requests 59 and 62 on Gitub/openssl; the pull request merged
by s0enke is at:
https://github.com/s0enke/openssl/pull/1
According to the "Contribute" section in the README, I write this
mail and attach a unified diff for patching the original
repository.
I admit that I myself just compiled it but did not run it. I have
no information what s0enke did.
Thanks for you interest so far.
Bertram Scharpf
--
Bertram Scharpf
Bopserstraße 15, D-70180 Stuttgart
Ruf 0711/2599337, Fax 0711/2184509 [13*79*2531,prim]
http://www.bertram-scharpf.de
diff --git a/ssl/Makefile b/ssl/Makefile
index e5df585..a88e0d1 100644
--- a/ssl/Makefile
+++ b/ssl/Makefile
@@ -30,7 +30,8 @@ LIBSRC= \
ssl_lib.c ssl_err2.c ssl_cert.c ssl_sess.c \
ssl_ciph.c ssl_stat.c ssl_rsa.c \
ssl_asn1.c ssl_txt.c ssl_algs.c ssl_conf.c \
- bio_ssl.c ssl_err.c kssl.c t1_reneg.c tls_srp.c t1_trce.c
+ bio_ssl.c ssl_err.c kssl.c t1_reneg.c tls_srp.c t1_trce.c \
+ heartbeat.c
LIBOBJ= \
s2_meth.o s2_srvr.o s2_clnt.o s2_lib.o s2_enc.o s2_pkt.o \
s3_meth.o s3_srvr.o s3_clnt.o s3_lib.o s3_enc.o s3_pkt.o s3_both.o s3_cbc.o \
@@ -41,7 +42,8 @@ LIBOBJ= \
ssl_lib.o ssl_err2.o ssl_cert.o ssl_sess.o \
ssl_ciph.o ssl_stat.o ssl_rsa.o \
ssl_asn1.o ssl_txt.o ssl_algs.o ssl_conf.o \
- bio_ssl.o ssl_err.o kssl.o t1_reneg.o tls_srp.o t1_trce.o
+ bio_ssl.o ssl_err.o kssl.o t1_reneg.o tls_srp.o t1_trce.o \
+ heartbeat.o
SRC= $(LIBSRC)
diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index d8bcd58..bdf7c94 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -1330,35 +1330,27 @@ dtls1_process_heartbeat(SSL *s)
unsigned int payload;
unsigned int padding = 16; /* Use minimum padding */
- if (s->msg_callback)
- s->msg_callback(0, s->version, TLS1_RT_HEARTBEAT,
- &s->s3->rrec.data[0], s->s3->rrec.length,
- s, s->msg_callback_arg);
+ apply_msg_callback(s);
- /* Read type and payload length first */
- if (1 + 2 + 16 > s->s3->rrec.length)
+ /* Read type and payload length */
+ if (heartbeat_size_std(0) > s->s3->rrec.length)
return 0; /* silently discard */
hbtype = *p++;
n2s(p, payload);
- if (1 + 2 + payload + 16 > s->s3->rrec.length)
+ if (heartbeat_size_std(payload) > s->s3->rrec.length)
return 0; /* silently discard per RFC 6520 sec. 4 */
pl = p;
if (hbtype == TLS1_HB_REQUEST)
{
unsigned char *buffer, *bp;
- unsigned int write_length = 1 /* heartbeat type */ +
- 2 /* heartbeat length */ +
- payload + padding;
+ unsigned int write_length = heartbeat_size(payload, padding);
int r;
if (write_length > SSL3_RT_MAX_PLAIN_LENGTH)
return 0;
- /* Allocate memory for the response, size is 1 byte
- * message type, plus 2 bytes payload length, plus
- * payload, plus padding
- */
+ /* Allocate memory for the response. */
buffer = OPENSSL_malloc(write_length);
bp = buffer;
@@ -1409,6 +1401,7 @@ dtls1_heartbeat(SSL *s)
int ret;
unsigned int payload = 18; /* Sequence number + random bytes */
unsigned int padding = 16; /* Use minimum padding */
+ unsigned int size;
/* Only send if peer supports and accepts HB requests... */
if (!(s->tlsext_heartbeat & SSL_TLSEXT_HB_ENABLED) ||
@@ -1440,13 +1433,9 @@ dtls1_heartbeat(SSL *s)
/* Create HeartBeat message, we just use a sequence number
* as payload to distuingish different messages and add
* some random stuff.
- * - Message Type, 1 byte
- * - Payload Length, 2 bytes (unsigned int)
- * - Payload, the sequence number (2 bytes uint)
- * - Payload, random bytes (16 bytes uint)
- * - Padding
*/
- buf = OPENSSL_malloc(1 + 2 + payload + padding);
+ size = heartbeat_size(payload, padding);
+ buf = OPENSSL_malloc(size);
p = buf;
/* Message Type */
*p++ = TLS1_HB_REQUEST;
@@ -1460,12 +1449,12 @@ dtls1_heartbeat(SSL *s)
/* Random padding */
RAND_pseudo_bytes(p, padding);
- ret = dtls1_write_bytes(s, TLS1_RT_HEARTBEAT, buf, 3 + payload + padding);
+ ret = dtls1_write_bytes(s, TLS1_RT_HEARTBEAT, buf, size);
if (ret >= 0)
{
if (s->msg_callback)
s->msg_callback(1, s->version, TLS1_RT_HEARTBEAT,
- buf, 3 + payload + padding,
+ buf, size,
s, s->msg_callback_arg);
dtls1_start_timer(s);
diff --git a/ssl/heartbeat.c b/ssl/heartbeat.c
new file mode 100644
index 0000000..eae91d3
--- /dev/null
+++ b/ssl/heartbeat.c
@@ -0,0 +1,22 @@
+#include "heartbeat.h"
+
+int heartbeat_size(int payload, int padding)
+ {
+ return 1 /* heartbeat type */ +
+ 2 /* heartbeat length */ +
+ payload + padding;
+ }
+
+int heartbeat_size_std(int payload)
+ {
+ return heartbeat_size(payload, 16);
+ }
+
+void apply_msg_callback(SSL *s)
+ {
+ if (s->msg_callback)
+ s->msg_callback(0, s->version, TLS1_RT_HEARTBEAT,
+ &s->s3->rrec.data[0], s->s3->rrec.length,
+ s, s->msg_callback_arg);
+ }
+
diff --git a/ssl/heartbeat.h b/ssl/heartbeat.h
new file mode 100644
index 0000000..24b9a01
--- /dev/null
+++ b/ssl/heartbeat.h
@@ -0,0 +1,11 @@
+#ifndef _INCL_HEARTBEAT_H
+#define _INCL_HEARTBEAT_H
+
+#include <openssl/ssl.h>
+
+int heartbeat_size(int payload, int padding);
+int heartbeat_size_std(int payload);
+
+void apply_msg_callback(SSL *s);
+
+#endif
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 0f51594..3498d69 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -165,6 +165,10 @@
#include <openssl/ssl.h>
#include <openssl/symhacks.h>
+#ifndef OPENSSL_NO_HEARTBEATS
+#include "heartbeat.h"
+#endif
+
#ifdef OPENSSL_BUILD_SHLIBSSL
# undef OPENSSL_EXTERN
# define OPENSSL_EXTERN OPENSSL_EXPORT
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index bcb99b8..cd64165 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -3968,18 +3968,16 @@ tls1_process_heartbeat(SSL *s)
unsigned short hbtype;
unsigned int payload;
unsigned int padding = 16; /* Use minimum padding */
+ unsigned int size;
- if (s->msg_callback)
- s->msg_callback(0, s->version, TLS1_RT_HEARTBEAT,
- &s->s3->rrec.data[0], s->s3->rrec.length,
- s, s->msg_callback_arg);
+ apply_msg_callback(s);
- /* Read type and payload length first */
- if (1 + 2 + 16 > s->s3->rrec.length)
+ /* Read type and payload length */
+ if (heartbeat_size_std(0) > s->s3->rrec.length)
return 0; /* silently discard */
hbtype = *p++;
n2s(p, payload);
- if (1 + 2 + payload + 16 > s->s3->rrec.length)
+ if (heartbeat_size_std(payload) > s->s3->rrec.length)
return 0; /* silently discard per RFC 6520 sec. 4 */
pl = p;
@@ -3988,11 +3986,9 @@ tls1_process_heartbeat(SSL *s)
unsigned char *buffer, *bp;
int r;
- /* Allocate memory for the response, size is 1 bytes
- * message type, plus 2 bytes payload length, plus
- * payload, plus padding
- */
- buffer = OPENSSL_malloc(1 + 2 + payload + padding);
+ /* Allocate memory for the response. */
+ size = heartbeat_size(payload, padding);
+ buffer = OPENSSL_malloc(size);
bp = buffer;
/* Enter response type, length and copy payload */
@@ -4003,11 +3999,11 @@ tls1_process_heartbeat(SSL *s)
/* Random padding */
RAND_pseudo_bytes(bp, padding);
- r = ssl3_write_bytes(s, TLS1_RT_HEARTBEAT, buffer, 3 + payload + padding);
+ r = ssl3_write_bytes(s, TLS1_RT_HEARTBEAT, buffer, size);
if (r >= 0 && s->msg_callback)
s->msg_callback(1, s->version, TLS1_RT_HEARTBEAT,
- buffer, 3 + payload + padding,
+ buffer, size,
s, s->msg_callback_arg);
OPENSSL_free(buffer);
@@ -4041,6 +4037,7 @@ tls1_heartbeat(SSL *s)
int ret;
unsigned int payload = 18; /* Sequence number + random bytes */
unsigned int padding = 16; /* Use minimum padding */
+ unsigned int size;
/* Only send if peer supports and accepts HB requests... */
if (!(s->tlsext_heartbeat & SSL_TLSEXT_HB_ENABLED) ||
@@ -4072,13 +4069,9 @@ tls1_heartbeat(SSL *s)
/* Create HeartBeat message, we just use a sequence number
* as payload to distuingish different messages and add
* some random stuff.
- * - Message Type, 1 byte
- * - Payload Length, 2 bytes (unsigned int)
- * - Payload, the sequence number (2 bytes uint)
- * - Payload, random bytes (16 bytes uint)
- * - Padding
*/
- buf = OPENSSL_malloc(1 + 2 + payload + padding);
+ size = heartbeat_size(payload, padding);
+ buf = OPENSSL_malloc(size);
p = buf;
/* Message Type */
*p++ = TLS1_HB_REQUEST;
@@ -4092,12 +4085,12 @@ tls1_heartbeat(SSL *s)
/* Random padding */
RAND_pseudo_bytes(p, padding);
- ret = ssl3_write_bytes(s, TLS1_RT_HEARTBEAT, buf, 3 + payload + padding);
+ ret = ssl3_write_bytes(s, TLS1_RT_HEARTBEAT, buf, size);
if (ret >= 0)
{
if (s->msg_callback)
s->msg_callback(1, s->version, TLS1_RT_HEARTBEAT,
- buf, 3 + payload + padding,
+ buf, size,
s, s->msg_callback_arg);
s->tlsext_hb_pending = 1;
