Commit 72df35ac authored by Matt Caswell's avatar Matt Caswell
Browse files

Tighten extension handling



This adds additional checks to the processing of extensions in a ClientHello
to ensure that either no extensions are present, or if they are then they
take up the exact amount of space expected.

With thanks to the Open Crypto Audit Project for reporting this issue.

Reviewed-by: default avatarStephen Henson <steve@openssl.org>

Conflicts:
	ssl/t1_lib.c
parent f92b1967
Loading
Loading
Loading
Loading
+64 −94
Original line number Diff line number Diff line
@@ -1016,19 +1016,23 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,

    s->srtp_profile = NULL;

    if (data >= (d + n - 2))
    if (data >= (d + n - 2)) {
        if (data != d + n)
            goto err;
        else
            goto ri_check;
    }
    n2s(data, len);

    if (data > (d + n - len))
        goto ri_check;
        goto err;

    while (data <= (d + n - 4)) {
        n2s(data, type);
        n2s(data, size);

        if (data + size > (d + n))
            goto ri_check;
            goto err;
# if 0
        fprintf(stderr, "Received extension type %d size %d\n", type, size);
# endif
@@ -1064,16 +1068,12 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
            int servname_type;
            int dsize;

            if (size < 2) {
                *al = SSL_AD_DECODE_ERROR;
                return 0;
            }
            if (size < 2)
                goto err;
            n2s(data, dsize);
            size -= 2;
            if (dsize > size) {
                *al = SSL_AD_DECODE_ERROR;
                return 0;
            }
            if (dsize > size)
                goto err;

            sdata = data;
            while (dsize > 3) {
@@ -1081,18 +1081,16 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
                n2s(sdata, len);
                dsize -= 3;

                if (len > dsize) {
                    *al = SSL_AD_DECODE_ERROR;
                    return 0;
                }
                if (len > dsize)
                    goto err;

                if (s->servername_done == 0)
                    switch (servname_type) {
                    case TLSEXT_NAMETYPE_host_name:
                        if (!s->hit) {
                            if (s->session->tlsext_hostname) {
                                *al = SSL_AD_DECODE_ERROR;
                                return 0;
                            }
                            if (s->session->tlsext_hostname)
                                goto err;

                            if (len > TLSEXT_MAXLEN_host_name) {
                                *al = TLS1_AD_UNRECOGNIZED_NAME;
                                return 0;
@@ -1126,31 +1124,23 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,

                dsize -= len;
            }
            if (dsize != 0) {
                *al = SSL_AD_DECODE_ERROR;
                return 0;
            }
            if (dsize != 0)
                goto err;

        }
# ifndef OPENSSL_NO_SRP
        else if (type == TLSEXT_TYPE_srp) {
            if (size == 0 || ((len = data[0])) != (size - 1)) {
                *al = SSL_AD_DECODE_ERROR;
                return 0;
            }
            if (s->srp_ctx.login != NULL) {
                *al = SSL_AD_DECODE_ERROR;
                return 0;
            }
            if (size == 0 || ((len = data[0])) != (size - 1))
                goto err;
            if (s->srp_ctx.login != NULL)
                goto err;
            if ((s->srp_ctx.login = OPENSSL_malloc(len + 1)) == NULL)
                return -1;
            memcpy(s->srp_ctx.login, &data[1], len);
            s->srp_ctx.login[len] = '\0';

            if (strlen(s->srp_ctx.login) != len) {
                *al = SSL_AD_DECODE_ERROR;
                return 0;
            }
            if (strlen(s->srp_ctx.login) != len)
                goto err;
        }
# endif

@@ -1159,10 +1149,8 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
            unsigned char *sdata = data;
            int ecpointformatlist_length = *(sdata++);

            if (ecpointformatlist_length != size - 1) {
                *al = TLS1_AD_DECODE_ERROR;
                return 0;
            }
            if (ecpointformatlist_length != size - 1)
                goto err;
            if (!s->hit) {
                if (s->session->tlsext_ecpointformatlist) {
                    OPENSSL_free(s->session->tlsext_ecpointformatlist);
@@ -1196,15 +1184,13 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
            if (ellipticcurvelist_length != size - 2 ||
                ellipticcurvelist_length < 1 ||
                /* Each NamedCurve is 2 bytes. */
                ellipticcurvelist_length & 1) {
                *al = TLS1_AD_DECODE_ERROR;
                return 0;
            }
                ellipticcurvelist_length & 1)
                    goto err;

            if (!s->hit) {
                if (s->session->tlsext_ellipticcurvelist) {
                    *al = TLS1_AD_DECODE_ERROR;
                    return 0;
                }
                if (s->session->tlsext_ellipticcurvelist)
                    goto err;

                s->session->tlsext_ellipticcurvelist_length = 0;
                if ((s->session->tlsext_ellipticcurvelist =
                     OPENSSL_malloc(ellipticcurvelist_length)) == NULL) {
@@ -1273,28 +1259,20 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
            renegotiate_seen = 1;
        } else if (type == TLSEXT_TYPE_signature_algorithms) {
            int dsize;
            if (sigalg_seen || size < 2) {
                *al = SSL_AD_DECODE_ERROR;
                return 0;
            }
            if (sigalg_seen || size < 2)
                goto err;
            sigalg_seen = 1;
            n2s(data, dsize);
            size -= 2;
            if (dsize != size || dsize & 1) {
                *al = SSL_AD_DECODE_ERROR;
                return 0;
            }
            if (!tls1_process_sigalgs(s, data, dsize)) {
                *al = SSL_AD_DECODE_ERROR;
                return 0;
            }
            if (dsize != size || dsize & 1)
                goto err;
            if (!tls1_process_sigalgs(s, data, dsize))
                goto err;
        } else if (type == TLSEXT_TYPE_status_request &&
                   s->version != DTLS1_VERSION) {

            if (size < 5) {
                *al = SSL_AD_DECODE_ERROR;
                return 0;
            }
            if (size < 5)
                goto err;

            s->tlsext_status_type = *data++;
            size--;
@@ -1304,35 +1282,26 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
                /* Read in responder_id_list */
                n2s(data, dsize);
                size -= 2;
                if (dsize > size) {
                    *al = SSL_AD_DECODE_ERROR;
                    return 0;
                }
                if (dsize > size)
                    goto err;
                while (dsize > 0) {
                    OCSP_RESPID *id;
                    int idsize;
                    if (dsize < 4) {
                        *al = SSL_AD_DECODE_ERROR;
                        return 0;
                    }
                    if (dsize < 4)
                        goto err;
                    n2s(data, idsize);
                    dsize -= 2 + idsize;
                    size -= 2 + idsize;
                    if (dsize < 0) {
                        *al = SSL_AD_DECODE_ERROR;
                        return 0;
                    }
                    if (dsize < 0)
                        goto err;
                    sdata = data;
                    data += idsize;
                    id = d2i_OCSP_RESPID(NULL, &sdata, idsize);
                    if (!id) {
                        *al = SSL_AD_DECODE_ERROR;
                        return 0;
                    }
                    if (!id)
                        goto err;
                    if (data != sdata) {
                        OCSP_RESPID_free(id);
                        *al = SSL_AD_DECODE_ERROR;
                        return 0;
                        goto err;
                    }
                    if (!s->tlsext_ocsp_ids
                        && !(s->tlsext_ocsp_ids =
@@ -1349,16 +1318,12 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
                }

                /* Read in request_extensions */
                if (size < 2) {
                    *al = SSL_AD_DECODE_ERROR;
                    return 0;
                }
                if (size < 2)
                    goto err;
                n2s(data, dsize);
                size -= 2;
                if (dsize != size) {
                    *al = SSL_AD_DECODE_ERROR;
                    return 0;
                }
                if (dsize != size)
                    goto err;
                sdata = data;
                if (dsize > 0) {
                    if (s->tlsext_ocsp_exts) {
@@ -1368,10 +1333,8 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,

                    s->tlsext_ocsp_exts =
                        d2i_X509_EXTENSIONS(NULL, &sdata, dsize);
                    if (!s->tlsext_ocsp_exts || (data + dsize != sdata)) {
                        *al = SSL_AD_DECODE_ERROR;
                        return 0;
                    }
                    if (!s->tlsext_ocsp_exts || (data + dsize != sdata))
                        goto err;
                }
            }
            /*
@@ -1432,6 +1395,10 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
        data += size;
    }

    /* Spurious data on the end */
    if (data != d + n)
        goto err;

    *p = data;

 ri_check:
@@ -1447,6 +1414,9 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
    }

    return 1;
err:
    *al = SSL_AD_DECODE_ERROR;
    return 0;
}

# ifndef OPENSSL_NO_NEXTPROTONEG