Fixed out-of-bounds read errors in ssl3_get_key_exchange.
PR#3450 Conflicts: ssl/s3_clnt.c Reviewed-by: Emilia Käsper <emilia@openssl.org>
This commit is contained in:
parent
44a8fced97
commit
bf3e53a7fa
1 changed files with 83 additions and 30 deletions
113
ssl/s3_clnt.c
113
ssl/s3_clnt.c
|
@ -1095,8 +1095,8 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
#endif
|
||||
EVP_MD_CTX md_ctx;
|
||||
unsigned char *param,*p;
|
||||
int al,i,j,param_len,ok;
|
||||
long n,alg;
|
||||
int al,j,ok;
|
||||
long i,param_len,n,alg;
|
||||
EVP_PKEY *pkey=NULL;
|
||||
#ifndef OPENSSL_NO_RSA
|
||||
RSA *rsa=NULL;
|
||||
|
@ -1160,10 +1160,12 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
s->session->sess_cert=ssl_sess_cert_new();
|
||||
}
|
||||
|
||||
/* Total length of the parameters including the length prefix */
|
||||
param_len=0;
|
||||
alg=s->s3->tmp.new_cipher->algorithms;
|
||||
EVP_MD_CTX_init(&md_ctx);
|
||||
|
||||
al=SSL_AD_DECODE_ERROR;
|
||||
#ifndef OPENSSL_NO_RSA
|
||||
if (alg & SSL_kRSA)
|
||||
{
|
||||
|
@ -1172,14 +1174,23 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_MALLOC_FAILURE);
|
||||
goto err;
|
||||
}
|
||||
n2s(p,i);
|
||||
param_len=i+2;
|
||||
|
||||
param_len = 2;
|
||||
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);
|
||||
goto f_err;
|
||||
}
|
||||
param_len += i;
|
||||
|
||||
if (!(rsa->n=BN_bin2bn(p,i,rsa->n)))
|
||||
{
|
||||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB);
|
||||
|
@ -1187,14 +1198,23 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
}
|
||||
p+=i;
|
||||
|
||||
n2s(p,i);
|
||||
param_len+=i+2;
|
||||
if (param_len > n)
|
||||
if (2 > n - param_len)
|
||||
{
|
||||
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);
|
||||
goto f_err;
|
||||
}
|
||||
param_len += i;
|
||||
|
||||
if (!(rsa->e=BN_bin2bn(p,i,rsa->e)))
|
||||
{
|
||||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB);
|
||||
|
@ -1226,14 +1246,23 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_DH_LIB);
|
||||
goto err;
|
||||
}
|
||||
n2s(p,i);
|
||||
param_len=i+2;
|
||||
|
||||
param_len = 2;
|
||||
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);
|
||||
goto f_err;
|
||||
}
|
||||
param_len += i;
|
||||
|
||||
if (!(dh->p=BN_bin2bn(p,i,NULL)))
|
||||
{
|
||||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB);
|
||||
|
@ -1241,14 +1270,23 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
}
|
||||
p+=i;
|
||||
|
||||
n2s(p,i);
|
||||
param_len+=i+2;
|
||||
if (param_len > n)
|
||||
if (2 > n - param_len)
|
||||
{
|
||||
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);
|
||||
goto f_err;
|
||||
}
|
||||
param_len += i;
|
||||
|
||||
if (!(dh->g=BN_bin2bn(p,i,NULL)))
|
||||
{
|
||||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB);
|
||||
|
@ -1256,14 +1294,23 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
}
|
||||
p+=i;
|
||||
|
||||
n2s(p,i);
|
||||
param_len+=i+2;
|
||||
if (param_len > n)
|
||||
if (2 > n - param_len)
|
||||
{
|
||||
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);
|
||||
goto f_err;
|
||||
}
|
||||
param_len += i;
|
||||
|
||||
if (!(dh->pub_key=BN_bin2bn(p,i,NULL)))
|
||||
{
|
||||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,ERR_R_BN_LIB);
|
||||
|
@ -1315,12 +1362,19 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
*/
|
||||
|
||||
/* 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;
|
||||
if ((param_len > n) ||
|
||||
(*p != NAMED_CURVE_TYPE) ||
|
||||
((curve_nid = curve_id2nid(*(p + 2))) == 0))
|
||||
param_len=4;
|
||||
if (param_len > n)
|
||||
{
|
||||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
|
||||
SSL_R_LENGTH_TOO_SHORT);
|
||||
goto f_err;
|
||||
}
|
||||
|
||||
if ((*p != NAMED_CURVE_TYPE) ||
|
||||
((curve_nid = curve_id2nid(*(p + 2))) == 0))
|
||||
{
|
||||
al=SSL_AD_INTERNAL_ERROR;
|
||||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS);
|
||||
|
@ -1362,15 +1416,15 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
|
||||
encoded_pt_len = *p; /* length of encoded point */
|
||||
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,
|
||||
p, encoded_pt_len, bn_ctx) == 0))
|
||||
{
|
||||
al=SSL_AD_DECODE_ERROR;
|
||||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_ECPOINT);
|
||||
goto f_err;
|
||||
}
|
||||
param_len += encoded_pt_len;
|
||||
|
||||
n-=param_len;
|
||||
p+=encoded_pt_len;
|
||||
|
@ -1421,10 +1475,10 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
n-=2;
|
||||
j=EVP_PKEY_size(pkey);
|
||||
|
||||
/* Check signature length. If n is 0 then signature is empty */
|
||||
if ((i != n) || (n > j) || (n <= 0))
|
||||
{
|
||||
/* wrong packet length */
|
||||
al=SSL_AD_DECODE_ERROR;
|
||||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_WRONG_SIGNATURE_LENGTH);
|
||||
goto f_err;
|
||||
}
|
||||
|
@ -1518,7 +1572,6 @@ int ssl3_get_key_exchange(SSL *s)
|
|||
}
|
||||
if (n != 0)
|
||||
{
|
||||
al=SSL_AD_DECODE_ERROR;
|
||||
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_EXTRA_DATA_IN_MESSAGE);
|
||||
goto f_err;
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue