[openssl-commits] [openssl] OpenSSL_1_0_2-stable update
Matt Caswell
matt at openssl.org
Wed Apr 29 16:45:46 UTC 2015
The branch OpenSSL_1_0_2-stable has been updated
via c5f8cd7bc661f90dc012c9d2bae1808a4281985f (commit)
from 937a766982229fd4aa3d9ceb544517f81a193206 (commit)
- Log -----------------------------------------------------------------
commit c5f8cd7bc661f90dc012c9d2bae1808a4281985f
Author: Matt Caswell <matt at openssl.org>
Date: Wed Apr 29 16:15:40 2015 +0100
Add length sanity check in SSLv2 n_do_ssl_write()
Fortify flagged up a problem in n_do_ssl_write() in SSLv2. Analysing the
code I do not believe there is a real problem here. However the logic flows
are complicated enough that a sanity check of |len| is probably worthwhile.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Rich Salz <rsalz at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
ssl/s2_pkt.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/ssl/s2_pkt.c b/ssl/s2_pkt.c
index 614b9a3..7a61888 100644
--- a/ssl/s2_pkt.c
+++ b/ssl/s2_pkt.c
@@ -576,6 +576,20 @@ static int n_do_ssl_write(SSL *s, const unsigned char *buf, unsigned int len)
s->s2->padding = p;
s->s2->mac_data = &(s->s2->wbuf[3]);
s->s2->wact_data = &(s->s2->wbuf[3 + mac_size]);
+
+ /*
+ * It would be clearer to write this as follows:
+ * if (mac_size + len + p > SSL2_MAX_RECORD_LENGTH_2_BYTE_HEADER)
+ * However |len| is user input that could in theory be very large. We
+ * know |mac_size| and |p| are small, so to avoid any possibility of
+ * overflow we write it like this.
+ *
+ * In theory this should never fail because the logic above should have
+ * modified |len| if it is too big. But we are being cautious.
+ */
+ if (len > (SSL2_MAX_RECORD_LENGTH_2_BYTE_HEADER - (mac_size + p))) {
+ return -1;
+ }
/* we copy the data into s->s2->wbuf */
memcpy(s->s2->wact_data, buf, len);
if (p)
More information about the openssl-commits
mailing list