Commit 80a06268 authored by Matt Caswell's avatar Matt Caswell
Browse files

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: default avatarRich Salz <rsalz@openssl.org>
(cherry picked from commit c5f8cd7b)
parent e3dd33c2

ssl/s2_pkt.c

+14 −0
Original line number Diff line number Diff line
@@ -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)