Add some missing error failure checks.
Found while diagnosing some crashes and hangs in the malloc tests. This (and
the follow-up) get us further but does not quite let the malloc tests pass
quietly, even without valgrind. DTLS silently ignores some malloc failures
(confusion with silently dropping bad packets) which then translate to hangs.
Change-Id: Ief06a671e0973d09d2883432b89a86259e346653
Reviewed-on: https://boringssl-review.googlesource.com/3482
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index a34d221..ddfc1fe 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -296,7 +296,7 @@
return ssl3_do_write(s, SSL3_RT_CHANGE_CIPHER_SPEC);
}
-unsigned long ssl3_output_cert_chain(SSL *s, CERT_PKEY *cpk) {
+int ssl3_output_cert_chain(SSL *s, CERT_PKEY *cpk) {
uint8_t *p;
unsigned long l = 3 + SSL_HM_HEADER_LENGTH(s);
@@ -309,7 +309,7 @@
l2n3(l, p);
l += 3;
ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE, l);
- return l + SSL_HM_HEADER_LENGTH(s);
+ return 1;
}
/* Obtain handshake message of message type |msg_type| (any if |msg_type| == -1),
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index c51ba6d..c6c2b42 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1100,6 +1100,9 @@
* initialized |sess_cert|. */
if (s->session->sess_cert == NULL) {
s->session->sess_cert = ssl_sess_cert_new();
+ if (s->session->sess_cert == NULL) {
+ return -1;
+ }
}
/* TODO(davidben): This should be reset in one place with the rest of the
@@ -1128,6 +1131,9 @@
}
} else {
s->session->sess_cert = ssl_sess_cert_new();
+ if (s->session->sess_cert == NULL) {
+ return -1;
+ }
}
alg_k = s->s3->tmp.new_cipher->algorithm_mkey;
@@ -2182,8 +2188,10 @@
}
if (s->state == SSL3_ST_CW_CERT_C) {
- s->state = SSL3_ST_CW_CERT_D;
- ssl3_output_cert_chain(s, (s->s3->tmp.cert_req == 2) ? NULL : s->cert->key);
+ CERT_PKEY *cert_pkey = (s->s3->tmp.cert_req == 2) ? NULL : s->cert->key;
+ if (!ssl3_output_cert_chain(s, cert_pkey)) {
+ return -1;
+ }
}
/* SSL3_ST_CW_CERT_D */
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 3f89558..c020b3a 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -2163,6 +2163,9 @@
/* Filter out unsupported certificate types. */
pkey = X509_get_pubkey(peer);
+ if (pkey == NULL) {
+ goto err;
+ }
if (!(X509_certificate_type(peer, pkey) & EVP_PKT_SIGN) ||
(pkey->type != EVP_PKEY_RSA && pkey->type != EVP_PKEY_EC)) {
al = SSL_AD_UNSUPPORTED_CERTIFICATE;
@@ -2413,7 +2416,9 @@
return 0;
}
- ssl3_output_cert_chain(s, cpk);
+ if (!ssl3_output_cert_chain(s, cpk)) {
+ return 0;
+ }
s->state = SSL3_ST_SW_CERT_B;
}
@@ -2683,6 +2688,9 @@
BN_init(&y);
sig.r = BN_new();
sig.s = BN_new();
+ if (sig.r == NULL || sig.s == NULL) {
+ goto err;
+ }
p = CBS_data(&extension);
if (BN_bin2bn(p + 0, 32, &x) == NULL ||
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 348e2a5..de57330 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1677,11 +1677,6 @@
ctx->next_proto_select_cb_arg = arg;
}
-/* SSL_CTX_set_alpn_protos sets the ALPN protocol list on |ctx| to |protos|.
- * |protos| must be in wire-format (i.e. a series of non-empty, 8-bit
- * length-prefixed strings).
- *
- * Returns 0 on success. */
int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const uint8_t *protos,
unsigned protos_len) {
if (ctx->alpn_client_proto_list) {
@@ -1697,11 +1692,6 @@
return 0;
}
-/* SSL_set_alpn_protos sets the ALPN protocol list on |ssl| to |protos|.
- * |protos| must be in wire-format (i.e. a series of non-empty, 8-bit
- * length-prefixed strings).
- *
- * Returns 0 on success. */
int SSL_set_alpn_protos(SSL *ssl, const uint8_t *protos, unsigned protos_len) {
if (ssl->alpn_client_proto_list) {
OPENSSL_free(ssl->alpn_client_proto_list);
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index d63ddda..c7abe79 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -758,7 +758,7 @@
int ssl3_cert_verify_mac(SSL *s, int md_nid, uint8_t *p);
void ssl3_finish_mac(SSL *s, const uint8_t *buf, int len);
void ssl3_free_digest_list(SSL *s);
-unsigned long ssl3_output_cert_chain(SSL *s, CERT_PKEY *cpk);
+int ssl3_output_cert_chain(SSL *s, CERT_PKEY *cpk);
const SSL_CIPHER *ssl3_choose_cipher(
SSL *ssl, STACK_OF(SSL_CIPHER) * clnt,
struct ssl_cipher_preference_list_st *srvr);
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index fd0223d..a3d8925 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -737,7 +737,10 @@
}
EVP_MD_CTX_init(&ctx);
- EVP_MD_CTX_copy_ex(&ctx, d);
+ if (!EVP_MD_CTX_copy_ex(&ctx, d)) {
+ EVP_MD_CTX_cleanup(&ctx);
+ return 0;
+ }
EVP_DigestFinal_ex(&ctx, out, &ret);
EVP_MD_CTX_cleanup(&ctx);
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 19e52a4..967dad7 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2584,7 +2584,10 @@
if (s->s3->handshake_dgst[i] == NULL) {
continue;
}
- EVP_MD_CTX_copy_ex(&ctx, s->s3->handshake_dgst[i]);
+ if (!EVP_MD_CTX_copy_ex(&ctx, s->s3->handshake_dgst[i])) {
+ EVP_MD_CTX_cleanup(&ctx);
+ return 0;
+ }
EVP_DigestFinal_ex(&ctx, temp_digest, &temp_digest_len);
EVP_DigestUpdate(md, temp_digest, temp_digest_len);
}
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 90d142a..e552065 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -363,10 +363,14 @@
case SSL_ERROR_WANT_WRITE:
AsyncBioAllowWrite(async, 1);
return 1;
- case SSL_ERROR_WANT_CHANNEL_ID_LOOKUP:
- GetTestState(ssl)->channel_id =
- LoadPrivateKey(GetConfigPtr(ssl)->send_channel_id);
+ case SSL_ERROR_WANT_CHANNEL_ID_LOOKUP: {
+ ScopedEVP_PKEY pkey = LoadPrivateKey(GetConfigPtr(ssl)->send_channel_id);
+ if (!pkey) {
+ return 0;
+ }
+ GetTestState(ssl)->channel_id = std::move(pkey);
return 1;
+ }
case SSL_ERROR_WANT_X509_LOOKUP:
GetTestState(ssl)->cert_ready = true;
return 1;
@@ -455,12 +459,17 @@
}
}
}
- if (!config->host_name.empty()) {
- SSL_set_tlsext_host_name(ssl.get(), config->host_name.c_str());
+ if (!config->host_name.empty() &&
+ !SSL_set_tlsext_host_name(ssl.get(), config->host_name.c_str())) {
+ BIO_print_errors_fp(stdout);
+ return 1;
}
- if (!config->advertise_alpn.empty()) {
- SSL_set_alpn_protos(ssl.get(), (const uint8_t *)config->advertise_alpn.data(),
- config->advertise_alpn.size());
+ if (!config->advertise_alpn.empty() &&
+ SSL_set_alpn_protos(ssl.get(),
+ (const uint8_t *)config->advertise_alpn.data(),
+ config->advertise_alpn.size()) != 0) {
+ BIO_print_errors_fp(stdout);
+ return 1;
}
if (!config->psk.empty()) {
SSL_set_psk_client_callback(ssl.get(), PskClientCallback);
diff --git a/ssl/test/packeted_bio.cc b/ssl/test/packeted_bio.cc
index e003985..8f4f3de 100644
--- a/ssl/test/packeted_bio.cc
+++ b/ssl/test/packeted_bio.cc
@@ -186,6 +186,9 @@
ScopedBIO PacketedBioCreate(OPENSSL_timeval *out_timeout) {
ScopedBIO bio(BIO_new(&g_packeted_bio_method));
+ if (!bio) {
+ return nullptr;
+ }
bio->ptr = out_timeout;
return bio;
}
