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: Stephen Henson <steve@openssl.org> Conflicts: ssl/t1_lib.c
This commit is contained in:
Matt Caswell
parent
b3c02473f7
commit
d272599277
158
ssl/t1_lib.c
158
ssl/t1_lib.c
@ -2016,19 +2016,23 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
|
||||
s->srtp_profile = NULL;
|
||||
|
||||
if (data >= (d + n - 2))
|
||||
goto ri_check;
|
||||
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
|
||||
@ -2064,16 +2068,12 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
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) {
|
||||
@ -2081,18 +2081,16 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
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;
|
||||
@ -2126,31 +2124,23 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
|
||||
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
|
||||
|
||||
@ -2160,10 +2150,8 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
int ecpointformatlist_length = *(sdata++);
|
||||
|
||||
if (ecpointformatlist_length != size - 1 ||
|
||||
ecpointformatlist_length < 1) {
|
||||
*al = TLS1_AD_DECODE_ERROR;
|
||||
return 0;
|
||||
}
|
||||
ecpointformatlist_length < 1)
|
||||
goto err;
|
||||
if (!s->hit) {
|
||||
if (s->session->tlsext_ecpointformatlist) {
|
||||
OPENSSL_free(s->session->tlsext_ecpointformatlist);
|
||||
@ -2197,15 +2185,13 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
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) {
|
||||
@ -2273,26 +2259,18 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
renegotiate_seen = 1;
|
||||
} else if (type == TLSEXT_TYPE_signature_algorithms) {
|
||||
int dsize;
|
||||
if (s->cert->peer_sigalgs || size < 2) {
|
||||
*al = SSL_AD_DECODE_ERROR;
|
||||
return 0;
|
||||
}
|
||||
if (s->cert->peer_sigalgs || size < 2)
|
||||
goto err;
|
||||
n2s(data, dsize);
|
||||
size -= 2;
|
||||
if (dsize != size || dsize & 1 || !dsize) {
|
||||
*al = SSL_AD_DECODE_ERROR;
|
||||
return 0;
|
||||
}
|
||||
if (!tls1_save_sigalgs(s, data, dsize)) {
|
||||
*al = SSL_AD_DECODE_ERROR;
|
||||
return 0;
|
||||
}
|
||||
if (dsize != size || dsize & 1 || !dsize)
|
||||
goto err;
|
||||
if (!tls1_save_sigalgs(s, data, dsize))
|
||||
goto err;
|
||||
} else if (type == TLSEXT_TYPE_status_request) {
|
||||
|
||||
if (size < 5) {
|
||||
*al = SSL_AD_DECODE_ERROR;
|
||||
return 0;
|
||||
}
|
||||
if (size < 5)
|
||||
goto err;
|
||||
|
||||
s->tlsext_status_type = *data++;
|
||||
size--;
|
||||
@ -2302,35 +2280,26 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
/* 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 =
|
||||
@ -2347,16 +2316,12 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
}
|
||||
|
||||
/* 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) {
|
||||
@ -2366,10 +2331,8 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
|
||||
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;
|
||||
}
|
||||
}
|
||||
/*
|
||||
@ -2441,6 +2404,10 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
data += size;
|
||||
}
|
||||
|
||||
/* Spurious data on the end */
|
||||
if (data != d + n)
|
||||
goto err;
|
||||
|
||||
*p = data;
|
||||
|
||||
ri_check:
|
||||
@ -2456,6 +2423,9 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p,
|
||||
}
|
||||
|
||||
return 1;
|
||||
err:
|
||||
*al = SSL_AD_DECODE_ERROR;
|
||||
return 0;
|
||||
}
|
||||
|
||||
/*
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user