Fix some malloc test crashs.
This isn't exhaustive. There are still failures in some tests which probably
ought to get C++'d first.
Change-Id: Iac58df9d98cdfd94603d54374a531b2559df64c3
Reviewed-on: https://boringssl-review.googlesource.com/4795
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/bio/printf.c b/crypto/bio/printf.c
index 3638915..f51b396 100644
--- a/crypto/bio/printf.c
+++ b/crypto/bio/printf.c
@@ -64,6 +64,7 @@
#include <stdarg.h>
#include <stdio.h>
+#include <openssl/err.h>
#include <openssl/mem.h>
int BIO_printf(BIO *bio, const char *format, ...) {
@@ -94,9 +95,8 @@
out = OPENSSL_malloc(requested_len + 1);
out_malloced = 1;
if (out == NULL) {
- /* Unclear what can be done in this situation. OpenSSL has historically
- * crashed and that seems better than producing the wrong output. */
- abort();
+ OPENSSL_PUT_ERROR(BIO, BIO_printf, ERR_R_MALLOC_FAILURE);
+ return -1;
}
va_start(args, format);
out_len = vsnprintf(out, requested_len + 1, format, args);
diff --git a/crypto/bio/socket_helper.c b/crypto/bio/socket_helper.c
index 197c737..b1cdd1a 100644
--- a/crypto/bio/socket_helper.c
+++ b/crypto/bio/socket_helper.c
@@ -51,7 +51,7 @@
ret = getaddrinfo(hostname, port_str, &hint, &result);
if (ret != 0) {
OPENSSL_PUT_ERROR(SYS, getaddrinfo, 0);
- ERR_add_error_data(2, gai_strerror(ret));
+ ERR_add_error_data(1, gai_strerror(ret));
return 0;
}
diff --git a/crypto/bytestring/ber.c b/crypto/bytestring/ber.c
index 2729fa1..2a7df63 100644
--- a/crypto/bytestring/ber.c
+++ b/crypto/bytestring/ber.c
@@ -209,7 +209,9 @@
return 1;
}
- CBB_init(&cbb, CBS_len(in));
+ if (!CBB_init(&cbb, CBS_len(in))) {
+ return 0;
+ }
if (!cbs_convert_ber(in, &cbb, 0, 0, 0)) {
CBB_cleanup(&cbb);
return 0;
diff --git a/crypto/cipher/cipher.c b/crypto/cipher/cipher.c
index 1dcfd06..400c3f5 100644
--- a/crypto/cipher/cipher.c
+++ b/crypto/cipher/cipher.c
@@ -94,14 +94,13 @@
}
int EVP_CIPHER_CTX_cleanup(EVP_CIPHER_CTX *c) {
- if (c->cipher != NULL && c->cipher->cleanup) {
- c->cipher->cleanup(c);
- }
-
- if (c->cipher_data) {
+ if (c->cipher != NULL) {
+ if (c->cipher->cleanup) {
+ c->cipher->cleanup(c);
+ }
OPENSSL_cleanse(c->cipher_data, c->cipher->ctx_size);
- OPENSSL_free(c->cipher_data);
}
+ OPENSSL_free(c->cipher_data);
memset(c, 0, sizeof(EVP_CIPHER_CTX));
return 1;
@@ -165,6 +164,7 @@
if (ctx->cipher->ctx_size) {
ctx->cipher_data = OPENSSL_malloc(ctx->cipher->ctx_size);
if (!ctx->cipher_data) {
+ ctx->cipher = NULL;
OPENSSL_PUT_ERROR(CIPHER, EVP_CipherInit_ex, ERR_R_MALLOC_FAILURE);
return 0;
}
@@ -177,6 +177,7 @@
if (ctx->cipher->flags & EVP_CIPH_CTRL_INIT) {
if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_INIT, 0, NULL)) {
+ ctx->cipher = NULL;
OPENSSL_PUT_ERROR(CIPHER, EVP_CipherInit_ex, CIPHER_R_INITIALIZATION_ERROR);
return 0;
}
diff --git a/crypto/dsa/dsa_impl.c b/crypto/dsa/dsa_impl.c
index b7e1fd8..2ab8ba8 100644
--- a/crypto/dsa/dsa_impl.c
+++ b/crypto/dsa/dsa_impl.c
@@ -501,12 +501,16 @@
}
ctx = BN_CTX_new();
+ if (ctx == NULL) {
+ goto err;
+ }
+ BN_CTX_start(ctx);
+
mont = BN_MONT_CTX_new();
- if (ctx == NULL || mont == NULL) {
+ if (mont == NULL) {
goto err;
}
- BN_CTX_start(ctx);
r0 = BN_CTX_get(ctx);
g = BN_CTX_get(ctx);
W = BN_CTX_get(ctx);
@@ -516,7 +520,7 @@
p = BN_CTX_get(ctx);
test = BN_CTX_get(ctx);
- if (!BN_lshift(test, BN_value_one(), bits - 1)) {
+ if (test == NULL || !BN_lshift(test, BN_value_one(), bits - 1)) {
goto err;
}
diff --git a/crypto/dsa/dsa_test.c b/crypto/dsa/dsa_test.c
index 9b70dbe..8bdaaf4 100644
--- a/crypto/dsa/dsa_test.c
+++ b/crypto/dsa/dsa_test.c
@@ -238,8 +238,10 @@
goto end;
}
- DSA_generate_key(dsa);
- DSA_sign(0, fips_digest, sizeof(fips_digest), sig, &siglen, dsa);
+ if (!DSA_generate_key(dsa) ||
+ !DSA_sign(0, fips_digest, sizeof(fips_digest), sig, &siglen, dsa)) {
+ goto end;
+ }
if (DSA_verify(0, fips_digest, sizeof(fips_digest), sig, siglen, dsa) == 1) {
ok = 1;
} else {
diff --git a/crypto/ec/ec_test.cc b/crypto/ec/ec_test.cc
index 74685eb..5af42d5 100644
--- a/crypto/ec/ec_test.cc
+++ b/crypto/ec/ec_test.cc
@@ -125,6 +125,9 @@
}
ScopedOpenSSLString x_hex(BN_bn2hex(x.get()));
ScopedOpenSSLString y_hex(BN_bn2hex(y.get()));
+ if (!x_hex || !y_hex) {
+ return false;
+ }
if (0 != strcmp(
x_hex.get(),
"c81561ecf2e54edefe6617db1c7a34a70744ddb261f269b83dacfcd2ade5a681") ||
diff --git a/crypto/err/bio.errordata b/crypto/err/bio.errordata
index cd7286a..9f2af02 100644
--- a/crypto/err/bio.errordata
+++ b/crypto/err/bio.errordata
@@ -3,6 +3,7 @@
BIO,function,102,BIO_new
BIO,function,103,BIO_new_file
BIO,function,104,BIO_new_mem_buf
+BIO,function,118,BIO_printf
BIO,function,105,BIO_zero_copy_get_read_buf
BIO,function,106,BIO_zero_copy_get_read_buf_done
BIO,function,107,BIO_zero_copy_get_write_buf
diff --git a/crypto/lhash/lhash_test.c b/crypto/lhash/lhash_test.c
index cf5e99b..63748e7 100644
--- a/crypto/lhash/lhash_test.c
+++ b/crypto/lhash/lhash_test.c
@@ -123,6 +123,9 @@
CRYPTO_library_init();
lh = lh_new(NULL, NULL);
+ if (lh == NULL) {
+ return 1;
+ }
for (i = 0; i < 100000; i++) {
unsigned action;
diff --git a/crypto/modes/gcm_test.c b/crypto/modes/gcm_test.c
index 3548c81..a8819ea 100644
--- a/crypto/modes/gcm_test.c
+++ b/crypto/modes/gcm_test.c
@@ -347,6 +347,9 @@
}
out = OPENSSL_malloc(plaintext_len);
+ if (out == NULL) {
+ goto out;
+ }
if (AES_set_encrypt_key(key, key_len*8, &aes_key)) {
fprintf(stderr, "%u: AES_set_encrypt_key failed.\n", test_num);
goto out;
diff --git a/include/openssl/bio.h b/include/openssl/bio.h
index c0c430a..66a72a1 100644
--- a/include/openssl/bio.h
+++ b/include/openssl/bio.h
@@ -873,6 +873,7 @@
#define BIO_F_file_ctrl 115
#define BIO_F_file_read 116
#define BIO_F_mem_write 117
+#define BIO_F_BIO_printf 118
#define BIO_R_BAD_FOPEN_MODE 100
#define BIO_R_BROKEN_PIPE 101
#define BIO_R_CONNECT_ERROR 102
diff --git a/include/openssl/cipher.h b/include/openssl/cipher.h
index f1469a0..7f5fe04 100644
--- a/include/openssl/cipher.h
+++ b/include/openssl/cipher.h
@@ -520,6 +520,9 @@
int (*cipher)(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in,
size_t inl);
+ /* cleanup, if non-NULL, releases memory associated with the context. It is
+ * called if |EVP_CTRL_INIT| succeeds. Note that |init| may not have been
+ * called at this point. */
void (*cleanup)(EVP_CIPHER_CTX *);
int (*ctrl)(EVP_CIPHER_CTX *, int type, int arg, void *ptr);
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index cd41211..a7e49c9 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -239,8 +239,9 @@
OPENSSL_EXPORT const char *SSL_CIPHER_get_kx_name(const SSL_CIPHER *cipher);
/* SSL_CIPHER_get_rfc_name returns a newly-allocated string with the standard
- * name for |cipher|. For example, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256". The
- * caller is responsible for calling |OPENSSL_free| on the result. */
+ * name for |cipher| or NULL on error. For example,
+ * "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256". The caller is responsible for
+ * calling |OPENSSL_free| on the result. */
OPENSSL_EXPORT char *SSL_CIPHER_get_rfc_name(const SSL_CIPHER *cipher);
/* SSL_CIPHER_get_bits returns the strength, in bits, of |cipher|. If
diff --git a/ssl/pqueue/pqueue_test.c b/ssl/pqueue/pqueue_test.c
index cb688f7..5a68fc4 100644
--- a/ssl/pqueue/pqueue_test.c
+++ b/ssl/pqueue/pqueue_test.c
@@ -72,7 +72,7 @@
for (i = 0; i < NUM_ITEMS; i++) {
priority[7] = ordering[i];
item = pitem_new(priority, &ordering[i]);
- if (pqueue_insert(q, item) != item) {
+ if (item == NULL || pqueue_insert(q, item) != item) {
return 0;
}
}
@@ -82,7 +82,7 @@
for (i = 0; i < NUM_ITEMS; i++) {
priority[7] = ordering[i];
item = pitem_new(priority, &ordering[i]);
- if (pqueue_insert(q, item) != NULL) {
+ if (item == NULL || pqueue_insert(q, item) != NULL) {
return 0;
}
pitem_free(item);
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 7886304..4de50fd 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -431,6 +431,9 @@
return false;
}
ScopedOpenSSLString rfc_name(SSL_CIPHER_get_rfc_name(cipher));
+ if (!rfc_name) {
+ return false;
+ }
out->assign(rfc_name.get());
return true;
}
