generic-library/openssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Add length sanity check in SSLv2 n_do_ssl_write()

Browse Source 
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@openssl.org>
This commit is contained in:
Matt Caswell 2015-04-29 16:15:40 +01:00
parent 937a766982
commit c5f8cd7bc6
1 changed files with 14 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
ssl/s2_pkt.c
View File

@ -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->padding = p;
s->s2->mac_data = &(s->s2->wbuf[3]); s->s2->mac_data = &(s->s2->wbuf[3]);
s->s2->wact_data = &(s->s2->wbuf[3 + mac_size]); 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 */ /* we copy the data into s->s2->wbuf */
memcpy(s->s2->wact_data, buf, len); memcpy(s->s2->wact_data, buf, len);
if (p) if (p)