improved error checking and some fixes

PR: 1170
Submitted by: Yair Elharrar
Reviewed and edited by: Nils Larsch

crypto/asn1/a_bitstr.c

@@ -183,9 +183,11 @@ int ASN1_BIT_STRING_set_bit(ASN1_BIT_STRING *a, int n, int value)
 	iv= ~v;
 	if (!value) v=0;
 
+	if (a == NULL)
+		return 0;
+
 	a->flags&= ~(ASN1_STRING_FLAG_BITS_LEFT|0x07); /* clear, set on write */
 
-	if (a == NULL) return(0);
 	if ((a->length < (w+1)) || (a->data == NULL))
 		{
 		if (!value) return(1); /* Don't need to set */

crypto/asn1/t_pkey.c

@@ -198,6 +198,11 @@ int DSA_print(BIO *bp, const DSA *x, int off)
 
 	if (x->p)
 		buf_len = (size_t)BN_num_bytes(x->p);
+	else
+		{
+		DSAerr(DSA_F_DSA_PRINT,DSA_R_MISSING_PARAMETERS);
+		goto err;
+		}
 	if (x->q)
 		if (buf_len < (i = (size_t)BN_num_bytes(x->q)))
 			buf_len = i;
@@ -670,6 +675,11 @@ int DHparams_print(BIO *bp, const DH *x)
 
 	if (x->p)
 		buf_len = (size_t)BN_num_bytes(x->p);
+	else
+		{
+		reason = ERR_R_PASSED_NULL_PARAMETER;
+		goto err;
+		}
 	if (x->g)
 		if (buf_len < (i = (size_t)BN_num_bytes(x->g)))
 			buf_len = i;
@@ -728,6 +738,11 @@ int DSAparams_print(BIO *bp, const DSA *x)
 
 	if (x->p)
 		buf_len = (size_t)BN_num_bytes(x->p);
+	else
+		{
+		DSAerr(DSA_F_DSA_PRINT,DSA_R_MISSING_PARAMETERS);
+		goto err;
+		}
 	if (x->q)
 		if (buf_len < (i = (size_t)BN_num_bytes(x->q)))
 			buf_len = i;
@@ -737,7 +752,7 @@ int DSAparams_print(BIO *bp, const DSA *x)
 	m=(unsigned char *)OPENSSL_malloc(buf_len+10);
 	if (m == NULL)
 		{
-		reason=ERR_R_MALLOC_FAILURE;
+		DSAerr(DSA_F_DSA_PRINT,ERR_R_MALLOC_FAILURE);
 		goto err;
 		}
 
@@ -750,7 +765,6 @@ int DSAparams_print(BIO *bp, const DSA *x)
 	ret=1;
 err:
 	if (m != NULL) OPENSSL_free(m);
-	DSAerr(DSA_F_DSAPARAMS_PRINT,reason);
 	return(ret);
 	}
 

crypto/evp/p5_crpt.c

@@ -114,9 +114,14 @@ int PKCS5_PBE_keyivgen(EVP_CIPHER_CTX *cctx, const char *pass, int passlen,
 	const unsigned char *pbuf;
 
 	/* Extract useful info from parameter */
+	if (param == NULL || param->type != V_ASN1_SEQUENCE ||
+	    param->value.sequence == NULL) {
+		EVPerr(EVP_F_PKCS5_PBE_KEYIVGEN,EVP_R_DECODE_ERROR);
+		return 0;
+	}
+
 	pbuf = param->value.sequence->data;
-	if (!param || (param->type != V_ASN1_SEQUENCE) ||
-	   !(pbe = d2i_PBEPARAM (NULL, &pbuf, param->value.sequence->length))) {
+	if (!(pbe = d2i_PBEPARAM(NULL, &pbuf, param->value.sequence->length))) {
 		EVPerr(EVP_F_PKCS5_PBE_KEYIVGEN,EVP_R_DECODE_ERROR);
 		return 0;
 	}

crypto/evp/p5_crpt2.c

@@ -156,10 +156,15 @@ int PKCS5_v2_PBE_keyivgen(EVP_CIPHER_CTX *ctx, const char *pass, int passlen,
 	const EVP_CIPHER *cipher;
 	PBKDF2PARAM *kdf = NULL;
 
+	if (param == NULL || param->type != V_ASN1_SEQUENCE ||
+	    param->value.sequence == NULL) {
+		EVPerr(EVP_F_PKCS5_V2_PBE_KEYIVGEN,EVP_R_DECODE_ERROR);
+		return 0;
+	}
+
 	pbuf = param->value.sequence->data;
 	plen = param->value.sequence->length;
-	if(!param || (param->type != V_ASN1_SEQUENCE) ||
-				   !(pbe2 = d2i_PBE2PARAM(NULL, &pbuf, plen))) {
+	if(!(pbe2 = d2i_PBE2PARAM(NULL, &pbuf, plen))) {
 		EVPerr(EVP_F_PKCS5_V2_PBE_KEYIVGEN,EVP_R_DECODE_ERROR);
 		return 0;
 	}

crypto/ocsp/ocsp_cl.c

@@ -101,6 +101,8 @@ int OCSP_request_set1_name(OCSP_REQUEST *req, X509_NAME *nm)
 	{
 	GENERAL_NAME *gen;
 	gen = GENERAL_NAME_new();
+	if (gen == NULL)
+		return 0;
 	if (!X509_NAME_set(&gen->d.directoryName, nm))
 		{
 		GENERAL_NAME_free(gen);

crypto/pkcs12/p12_crpt.c

@@ -94,9 +94,14 @@ int PKCS12_PBE_keyivgen(EVP_CIPHER_CTX *ctx, const char *pass, int passlen,
 	unsigned char key[EVP_MAX_KEY_LENGTH], iv[EVP_MAX_IV_LENGTH];
 
 	/* Extract useful info from parameter */
+	if (param == NULL || param->type != V_ASN1_SEQUENCE ||
+	    param->value.sequence == NULL) {
+		PKCS12err(PKCS12_F_PKCS12_PBE_KEYIVGEN,PKCS12_R_DECODE_ERROR);
+		return 0;
+	}
+
 	pbuf = param->value.sequence->data;
-	if (!param || (param->type != V_ASN1_SEQUENCE) ||
-	   !(pbe = d2i_PBEPARAM (NULL, &pbuf, param->value.sequence->length))) {
+	if (!(pbe = d2i_PBEPARAM(NULL, &pbuf, param->value.sequence->length))) {
 		PKCS12err(PKCS12_F_PKCS12_PBE_KEYIVGEN,PKCS12_R_DECODE_ERROR);
 		return 0;
 	}

crypto/txt_db/txt_db.c

@@ -179,10 +179,13 @@ err:
 #if !defined(OPENSSL_NO_STDIO) && !defined(OPENSSL_SYS_WIN16)
 		if (er == 1) fprintf(stderr,"OPENSSL_malloc failure\n");
 #endif
-		if (ret->data != NULL) sk_free(ret->data);
-		if (ret->index != NULL) OPENSSL_free(ret->index);
-		if (ret->qual != NULL) OPENSSL_free(ret->qual);
-		if (ret != NULL) OPENSSL_free(ret);
+		if (ret != NULL)
+			{
+			if (ret->data != NULL) sk_free(ret->data);
+			if (ret->index != NULL) OPENSSL_free(ret->index);
+			if (ret->qual != NULL) OPENSSL_free(ret->qual);
+			if (ret != NULL) OPENSSL_free(ret);
+			}
 		return(NULL);
 		}
 	else

crypto/ui/ui_lib.c

@@ -620,8 +620,10 @@ UI_METHOD *UI_create_method(char *name)
 	UI_METHOD *ui_method = (UI_METHOD *)OPENSSL_malloc(sizeof(UI_METHOD));
 
 	if (ui_method)
+		{
 		memset(ui_method, 0, sizeof(*ui_method));
-	ui_method->name = BUF_strdup(name);
+		ui_method->name = BUF_strdup(name);
+		}
 	return ui_method;
 	}
 

crypto/x509/x509_att.c

@@ -125,7 +125,13 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr(STACK_OF(X509_ATTRIBUTE) **x,
 	X509_ATTRIBUTE *new_attr=NULL;
 	STACK_OF(X509_ATTRIBUTE) *sk=NULL;
 
-	if ((x != NULL) && (*x == NULL))
+	if (x == NULL)
+		{
+		X509err(X509_F_X509AT_ADD1_ATTR, ERR_R_PASSED_NULL_PARAMETER);
+		goto err2;
+		} 
+
+	if (*x == NULL)
 		{
 		if ((sk=sk_X509_ATTRIBUTE_new_null()) == NULL)
 			goto err;
@@ -137,7 +143,7 @@ STACK_OF(X509_ATTRIBUTE) *X509at_add1_attr(STACK_OF(X509_ATTRIBUTE) **x,
 		goto err2;
 	if (!sk_X509_ATTRIBUTE_push(sk,new_attr))
 		goto err;
-	if ((x != NULL) && (*x == NULL))
+	if (*x == NULL)
 		*x=sk;
 	return(sk);
 err:

crypto/x509/x509_v3.c

@@ -147,7 +147,13 @@ STACK_OF(X509_EXTENSION) *X509v3_add_ext(STACK_OF(X509_EXTENSION) **x,
 	int n;
 	STACK_OF(X509_EXTENSION) *sk=NULL;
 
-	if ((x != NULL) && (*x == NULL))
+	if (x == NULL)
+		{
+		X509err(X509_F_X509V3_ADD_EXT,ERR_R_PASSED_NULL_PARAMETER);
+		goto err2;
+		}
+
+	if (*x == NULL)
 		{
 		if ((sk=sk_X509_EXTENSION_new_null()) == NULL)
 			goto err;
@@ -163,7 +169,7 @@ STACK_OF(X509_EXTENSION) *X509v3_add_ext(STACK_OF(X509_EXTENSION) **x,
 		goto err2;
 	if (!sk_X509_EXTENSION_insert(sk,new_ex,loc))
 		goto err;
-	if ((x != NULL) && (*x == NULL))
+	if (*x == NULL)
 		*x=sk;
 	return(sk);
 err:

crypto/x509v3/v3_alt.c

@@ -341,7 +341,8 @@ static int copy_email(X509V3_CTX *ctx, GENERAL_NAMES *gens, int move_p)
 	X509_NAME_ENTRY *ne;
 	GENERAL_NAME *gen = NULL;
 	int i;
-	if(ctx->flags == CTX_TEST) return 1;
+	if(ctx != NULL && ctx->flags == CTX_TEST)
+		return 1;
 	if(!ctx || (!ctx->subject_cert && !ctx->subject_req)) {
 		X509V3err(X509V3_F_COPY_EMAIL,X509V3_R_NO_SUBJECT_DETAILS);
 		goto err;

ssl/ssl_txt.c

@@ -153,7 +153,7 @@ int SSL_SESSION_print(BIO *bp, const SSL_SESSION *x)
 #endif /* OPENSSL_NO_KRB5 */
 	if (x->compress_meth != 0)
 		{
-		SSL_COMP *comp;
+		SSL_COMP *comp = NULL;
 
 		ssl_cipher_get_evp(x,NULL,NULL,&comp);
 		if (comp == NULL)