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

Fixed out-of-bounds read errors in ssl3_get_key_exchange.

Browse Source 
PR#3450

Conflicts:
	ssl/s3_clnt.c

Reviewed-by: Emilia Käsper <emilia@openssl.org>
This commit is contained in:
Matt Caswell 2014-07-26 23:47:40 +01:00
parent 1d5e58b53c
commit 679d0c8655
1 changed files with 98 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

132
ssl/s3_clnt.c
View File

@ -1174,8 +1174,8 @@ int ssl3_get_key_exchange(SSL *s)
#endif #endif
EVP_MD_CTX md_ctx; EVP_MD_CTX md_ctx;
unsigned char *param,*p; unsigned char *param,*p;
int al,i,j,param_len,ok; int al,j,ok;
long n,alg_k,alg_a; long i,param_len,n,alg_k,alg_a;
EVP_PKEY *pkey=NULL; EVP_PKEY *pkey=NULL;
#ifndef OPENSSL_NO_RSA #ifndef OPENSSL_NO_RSA
RSA *rsa=NULL; RSA *rsa=NULL;
@ -1250,36 +1250,48 @@ int ssl3_get_key_exchange(SSL *s)
s->session->sess_cert=ssl_sess_cert_new(); s->session->sess_cert=ssl_sess_cert_new();
} }
/* Total length of the parameters including the length prefix */
param_len=0; param_len=0;
alg_k=s->s3->tmp.new_cipher->algorithm_mkey; alg_k=s->s3->tmp.new_cipher->algorithm_mkey;
alg_a=s->s3->tmp.new_cipher->algorithm_auth; alg_a=s->s3->tmp.new_cipher->algorithm_auth;
EVP_MD_CTX_init(&md_ctx); EVP_MD_CTX_init(&md_ctx);
al=SSL_AD_DECODE_ERROR;
#ifndef OPENSSL_NO_PSK #ifndef OPENSSL_NO_PSK
if (alg_k & SSL_kPSK) if (alg_k & SSL_kPSK)
{ {
char tmp_id_hint[PSK_MAX_IDENTITY_LEN+1]; char tmp_id_hint[PSK_MAX_IDENTITY_LEN+1];
al=SSL_AD_HANDSHAKE_FAILURE; param_len = 2;
if (param_len > n)
{
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
SSL_R_LENGTH_TOO_SHORT);
goto f_err;
}
n2s(p,i); n2s(p,i);
param_len=i+2;
/* Store PSK identity hint for later use, hint is used /* Store PSK identity hint for later use, hint is used
* in ssl3_send_client_key_exchange. Assume that the * in ssl3_send_client_key_exchange. Assume that the
* maximum length of a PSK identity hint can be as * maximum length of a PSK identity hint can be as
* long as the maximum length of a PSK identity. */ * long as the maximum length of a PSK identity. */
if (i > PSK_MAX_IDENTITY_LEN) if (i > PSK_MAX_IDENTITY_LEN)
{ {
al=SSL_AD_HANDSHAKE_FAILURE;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
SSL_R_DATA_LENGTH_TOO_LONG); SSL_R_DATA_LENGTH_TOO_LONG);
goto f_err; goto f_err;
} }
if (param_len > n) if (i > n - param_len)
{ {
al=SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
SSL_R_BAD_PSK_IDENTITY_HINT_LENGTH); SSL_R_BAD_PSK_IDENTITY_HINT_LENGTH);
goto f_err; goto f_err;
} }
param_len += i;
/* If received PSK identity hint contains NULL /* If received PSK identity hint contains NULL
* characters, the hint is truncated from the first * characters, the hint is truncated from the first
* NULL. p may not be ending with NULL, so create a * NULL. p may not be ending with NULL, so create a
@ -1291,6 +1303,7 @@ int ssl3_get_key_exchange(SSL *s)
s->ctx->psk_identity_hint = BUF_strdup(tmp_id_hint); s->ctx->psk_identity_hint = BUF_strdup(tmp_id_hint);
if (s->ctx->psk_identity_hint == NULL) if (s->ctx->psk_identity_hint == NULL)
{ {
al=SSL_AD_HANDSHAKE_FAILURE;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE);
goto f_err; goto f_err;
} }
@ -1308,14 +1321,23 @@ int ssl3_get_key_exchange(SSL *s)
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_MALLOC_FAILURE); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_MALLOC_FAILURE);
goto err; goto err;
} }
n2s(p,i);
param_len=i+2; param_len = 2;
if (param_len > n) if (param_len > n)
{ {
al=SSL_AD_DECODE_ERROR; SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
SSL_R_LENGTH_TOO_SHORT);
goto f_err;
}
n2s(p,i);
if (i > n - param_len)
{
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_RSA_MODULUS_LENGTH); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_RSA_MODULUS_LENGTH);
goto f_err; goto f_err;
} }
param_len += i;
if (!(rsa->n=BN_bin2bn(p,i,rsa->n))) if (!(rsa->n=BN_bin2bn(p,i,rsa->n)))
{ {
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB);
@ -1323,14 +1345,23 @@ int ssl3_get_key_exchange(SSL *s)
} }
p+=i; p+=i;
n2s(p,i); if (2 > n - param_len)
param_len+=i+2; {
if (param_len > n) SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
SSL_R_LENGTH_TOO_SHORT);
goto f_err;
}
param_len += 2;
n2s(p,i);
if (i > n - param_len)
{ {
al=SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_RSA_E_LENGTH); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_RSA_E_LENGTH);
goto f_err; goto f_err;
} }
param_len += i;
if (!(rsa->e=BN_bin2bn(p,i,rsa->e))) if (!(rsa->e=BN_bin2bn(p,i,rsa->e)))
{ {
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB);
@ -1362,14 +1393,23 @@ int ssl3_get_key_exchange(SSL *s)
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_DH_LIB); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_DH_LIB);
goto err; goto err;
} }
n2s(p,i);
param_len=i+2; param_len = 2;
if (param_len > n) if (param_len > n)
{ {
al=SSL_AD_DECODE_ERROR; SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
SSL_R_LENGTH_TOO_SHORT);
goto f_err;
}
n2s(p,i);
if (i > n - param_len)
{
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_DH_P_LENGTH); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_DH_P_LENGTH);
goto f_err; goto f_err;
} }
param_len += i;
if (!(dh->p=BN_bin2bn(p,i,NULL))) if (!(dh->p=BN_bin2bn(p,i,NULL)))
{ {
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB);
@ -1377,14 +1417,23 @@ int ssl3_get_key_exchange(SSL *s)
} }
p+=i; p+=i;
n2s(p,i); if (2 > n - param_len)
param_len+=i+2; {
if (param_len > n) SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
SSL_R_LENGTH_TOO_SHORT);
goto f_err;
}
param_len += 2;
n2s(p,i);
if (i > n - param_len)
{ {
al=SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_DH_G_LENGTH); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_DH_G_LENGTH);
goto f_err; goto f_err;
} }
param_len += i;
if (!(dh->g=BN_bin2bn(p,i,NULL))) if (!(dh->g=BN_bin2bn(p,i,NULL)))
{ {
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB);
@ -1392,14 +1441,23 @@ int ssl3_get_key_exchange(SSL *s)
} }
p+=i; p+=i;
n2s(p,i); if (2 > n - param_len)
param_len+=i+2; {
if (param_len > n) SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
SSL_R_LENGTH_TOO_SHORT);
goto f_err;
}
param_len += 2;
n2s(p,i);
if (i > n - param_len)
{ {
al=SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_DH_PUB_KEY_LENGTH); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_DH_PUB_KEY_LENGTH);
goto f_err; goto f_err;
} }
param_len += i;
if (!(dh->pub_key=BN_bin2bn(p,i,NULL))) if (!(dh->pub_key=BN_bin2bn(p,i,NULL)))
{ {
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB);
@ -1451,12 +1509,19 @@ int ssl3_get_key_exchange(SSL *s)
*/ */
/* XXX: For now we only support named (not generic) curves /* XXX: For now we only support named (not generic) curves
* and the ECParameters in this case is just three bytes. * and the ECParameters in this case is just three bytes. We
* also need one byte for the length of the encoded point
*/ */
param_len=3; param_len=4;
if ((param_len > n) || if (param_len > n)
(*p != NAMED_CURVE_TYPE) || {
((curve_nid = tls1_ec_curve_id2nid(*(p + 2))) == 0)) SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
SSL_R_LENGTH_TOO_SHORT);
goto f_err;
}
if ((*p != NAMED_CURVE_TYPE) ||
((curve_nid = tls1_ec_curve_id2nid(*(p + 2))) == 0))
{ {
al=SSL_AD_INTERNAL_ERROR; al=SSL_AD_INTERNAL_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS);
@ -1498,15 +1563,15 @@ int ssl3_get_key_exchange(SSL *s)
encoded_pt_len = *p; /* length of encoded point */ encoded_pt_len = *p; /* length of encoded point */
p+=1; p+=1;
param_len += (1 + encoded_pt_len);
if ((param_len > n) || if ((encoded_pt_len > n - param_len) ||
(EC_POINT_oct2point(group, srvr_ecpoint, (EC_POINT_oct2point(group, srvr_ecpoint,
p, encoded_pt_len, bn_ctx) == 0)) p, encoded_pt_len, bn_ctx) == 0))
{ {
al=SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_ECPOINT); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_ECPOINT);
goto f_err; goto f_err;
} }
param_len += encoded_pt_len;
n-=param_len; n-=param_len;
p+=encoded_pt_len; p+=encoded_pt_len;
@ -1551,10 +1616,10 @@ int ssl3_get_key_exchange(SSL *s)
n-=2; n-=2;
j=EVP_PKEY_size(pkey); j=EVP_PKEY_size(pkey);
/* Check signature length. If n is 0 then signature is empty */
if ((i != n) || (n > j) || (n <= 0)) if ((i != n) || (n > j) || (n <= 0))
{ {
/* wrong packet length */ /* wrong packet length */
al=SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_WRONG_SIGNATURE_LENGTH); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_WRONG_SIGNATURE_LENGTH);
goto f_err; goto f_err;
} }
@ -1647,7 +1712,6 @@ int ssl3_get_key_exchange(SSL *s)
/* still data left over */ /* still data left over */
if (n != 0) if (n != 0)
{ {
al=SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_EXTRA_DATA_IN_MESSAGE); SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_EXTRA_DATA_IN_MESSAGE);
goto f_err; goto f_err;
} }