PR: 2295

Submitted by: Alexei Khlebnikov <alexei.khlebnikov@opera.com>
Reviewed by: steve

OOM checking. Leak in OOM fix. Fall-through comment. Duplicate code
elimination.

crypto/bn/bn_exp2.c

@@ -301,7 +301,8 @@ int BN_mod_exp2_mont(BIGNUM *rr, const BIGNUM *a1, const BIGNUM *p1,
 			r_is_one = 0;
 			}
 		}
-	BN_from_montgomery(rr,r,mont,ctx);
+	if (!BN_from_montgomery(rr,r,mont,ctx))
+		goto err;
 	ret=1;
 err:
 	if ((in_mont == NULL) && (mont != NULL)) BN_MONT_CTX_free(mont);

crypto/dsa/dsa_ossl.c

@@ -185,7 +185,7 @@ static DSA_SIG *dsa_do_sign(const unsigned char *dgst, int dlen, DSA *dsa)
 	if (!BN_mod_mul(&xr,dsa->priv_key,r,dsa->q,ctx)) goto err;/* s = xr */
 	if (!BN_add(s, &xr, &m)) goto err;		/* s = m + xr */
 	if (BN_cmp(s,dsa->q) > 0)
-		BN_sub(s,s,dsa->q);
+		if (!BN_sub(s,s,dsa->q)) goto err;
 	if (!BN_mod_mul(s,s,kinv,dsa->q,ctx)) goto err;
 
 	ret=DSA_SIG_new();

crypto/evp/evp_enc.c

@@ -204,6 +204,7 @@ skip_to_init:
 			case EVP_CIPH_OFB_MODE:
 
 			ctx->num = 0;
+			/* fall-through */
 
 			case EVP_CIPH_CBC_MODE:
 

crypto/pkcs12/p12_key.c

@@ -107,6 +107,7 @@ int PKCS12_key_gen_uni(unsigned char *pass, int passlen, unsigned char *salt,
 	unsigned char *B, *D, *I, *p, *Ai;
 	int Slen, Plen, Ilen, Ijlen;
 	int i, j, u, v;
+	int ret = 0;
 	BIGNUM *Ij, *Bpl1;	/* These hold Ij and B + 1 */
 	EVP_MD_CTX ctx;
 #ifdef  DEBUG_KEYGEN
@@ -144,10 +145,8 @@ int PKCS12_key_gen_uni(unsigned char *pass, int passlen, unsigned char *salt,
 	I = OPENSSL_malloc (Ilen);
 	Ij = BN_new();
 	Bpl1 = BN_new();
-	if (!D || !Ai || !B || !I || !Ij || !Bpl1) {
-		PKCS12err(PKCS12_F_PKCS12_KEY_GEN_UNI,ERR_R_MALLOC_FAILURE);
-		return 0;
-	}
+	if (!D || !Ai || !B || !I || !Ij || !Bpl1)
+		goto err;
 	for (i = 0; i < v; i++) D[i] = id;
 	p = I;
 	for (i = 0; i < Slen; i++) *p++ = salt[i % saltlen];
@@ -166,18 +165,12 @@ int PKCS12_key_gen_uni(unsigned char *pass, int passlen, unsigned char *salt,
 		}
 		memcpy (out, Ai, min (n, u));
 		if (u >= n) {
-			OPENSSL_free (Ai);
-			OPENSSL_free (B);
-			OPENSSL_free (D);
-			OPENSSL_free (I);
-			BN_free (Ij);
-			BN_free (Bpl1);
-			EVP_MD_CTX_cleanup(&ctx);
 #ifdef DEBUG_KEYGEN
 			fprintf(stderr, "Output KEY (length %d)\n", tmpn);
 			h__dump(tmpout, tmpn);
 #endif
-			return 1;	
+			ret = 1;
+			goto end;
 		}
 		n -= u;
 		out += u;
@@ -186,26 +179,41 @@ int PKCS12_key_gen_uni(unsigned char *pass, int passlen, unsigned char *salt,
 		BN_bin2bn (B, v, Bpl1);
 		BN_add_word (Bpl1, 1);
 		for (j = 0; j < Ilen ; j+=v) {
-			BN_bin2bn (I + j, v, Ij);
-			BN_add (Ij, Ij, Bpl1);
-			BN_bn2bin (Ij, B);
+			if (!BN_bin2bn(I + j, v, Ij))
+				goto err;
+			if (!BN_add(Ij, Ij, Bpl1))
+				goto err;
+			if (!BN_bn2bin(Ij, B))
+				goto err;
 			Ijlen = BN_num_bytes (Ij);
 			/* If more than 2^(v*8) - 1 cut off MSB */
 			if (Ijlen > v) {
-				BN_bn2bin (Ij, B);
+				if (!BN_bn2bin (Ij, B))
+					goto err;
 				memcpy (I + j, B + 1, v);
 #ifndef PKCS12_BROKEN_KEYGEN
 			/* If less than v bytes pad with zeroes */
 			} else if (Ijlen < v) {
 				memset(I + j, 0, v - Ijlen);
-				BN_bn2bin(Ij, I + j + v - Ijlen); 
+				if (!BN_bn2bin(Ij, I + j + v - Ijlen))
+					goto err;
 #endif
-			} else BN_bn2bin (Ij, I + j);
+			} else if (!BN_bn2bin (Ij, I + j))
+				goto err;
 		}
 	}
 	err:
+	PKCS12err(PKCS12_F_PKCS12_KEY_GEN_UNI,ERR_R_MALLOC_FAILURE);
+
+	end:
+	OPENSSL_free (Ai);
+	OPENSSL_free (B);
+	OPENSSL_free (D);
+	OPENSSL_free (I);
+	BN_free (Ij);
+	BN_free (Bpl1);
 	EVP_MD_CTX_cleanup(&ctx);
-	return 0;
+	return ret;
 }
 #ifdef DEBUG_KEYGEN
 void h__dump (unsigned char *p, int len)

crypto/rsa/rsa_eay.c

@@ -675,7 +675,7 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from,
 		rsa->_method_mod_n)) goto err;
 
 	if ((padding == RSA_X931_PADDING) && ((ret->d[0] & 0xf) != 12))
-		BN_sub(ret, rsa->n, ret);
+		if (!BN_sub(ret, rsa->n, ret)) goto err;
 
 	p=buf;
 	i=BN_bn2bin(ret,p);

crypto/x509v3/v3_ncons.c

@@ -189,7 +189,6 @@ static int do_i2r_name_constraints(const X509V3_EXT_METHOD *method,
 			print_nc_ipadd(bp, tree->base->d.ip);
 		else
 			GENERAL_NAME_print(bp, tree->base);
-		tree = sk_GENERAL_SUBTREE_value(trees, i);
 		BIO_puts(bp, "\n");
 		}
 	return 1;