From 4d1221e52ac0c8e887480dace397394fac5adf2f Mon Sep 17 00:00:00 2001 From: doug Date: Sat, 3 Jan 2015 18:07:29 +0000 Subject: [PATCH] Fix incorrect OPENSSL_assert() usage. Instead of asserting, return an error code for I/O errors. This is based on OpenSSL commit 2521fcd8527008ceb3e4748f95b0ed4e2d70cfef. Added checks for two calloc()s while I'm here. ok miod@ --- lib/libcrypto/bio/bss_dgram.c | 77 ++++++++++++++++++--------- lib/libssl/src/crypto/bio/bss_dgram.c | 77 ++++++++++++++++++--------- 2 files changed, 106 insertions(+), 48 deletions(-) diff --git a/lib/libcrypto/bio/bss_dgram.c b/lib/libcrypto/bio/bss_dgram.c index 2e17dc9e21d..c6b552eb322 100644 --- a/lib/libcrypto/bio/bss_dgram.c +++ b/lib/libcrypto/bio/bss_dgram.c @@ -1,4 +1,4 @@ -/* $OpenBSD: bss_dgram.c,v 1.37 2014/11/26 05:41:44 bcook Exp $ */ +/* $OpenBSD: bss_dgram.c,v 1.38 2015/01/03 18:07:29 doug Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -719,18 +719,25 @@ BIO_new_dgram_sctp(int fd, int close_flag) /* Activate SCTP-AUTH for DATA and FORWARD-TSN chunks */ auth.sauth_chunk = OPENSSL_SCTP_DATA_CHUNK_TYPE; ret = setsockopt(fd, IPPROTO_SCTP, SCTP_AUTH_CHUNK, &auth, sizeof(struct sctp_authchunk)); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; auth.sauth_chunk = OPENSSL_SCTP_FORWARD_CUM_TSN_CHUNK_TYPE; ret = setsockopt(fd, IPPROTO_SCTP, SCTP_AUTH_CHUNK, &auth, sizeof(struct sctp_authchunk)); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; /* Test if activation was successful. When using accept(), * SCTP-AUTH has to be activated for the listening socket * already, otherwise the connected socket won't use it. */ sockopt_len = (socklen_t)(sizeof(sctp_assoc_t) + 256 * sizeof(uint8_t)); authchunks = calloc(1, sockopt_len); + if (authchunks == NULL) + goto err; ret = getsockopt(fd, IPPROTO_SCTP, SCTP_LOCAL_AUTH_CHUNKS, authchunks, &sockopt_len); - OPENSSL_assert(ret >= 0); + if (ret < 0) { + free(authchunks); + goto err; + } for (p = (unsigned char*) authchunks->gauth_chunks; p < (unsigned char*) authchunks + sockopt_len; @@ -753,16 +760,19 @@ BIO_new_dgram_sctp(int fd, int close_flag) event.se_type = SCTP_AUTHENTICATION_EVENT; event.se_on = 1; ret = setsockopt(fd, IPPROTO_SCTP, SCTP_EVENT, &event, sizeof(struct sctp_event)); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; #else sockopt_len = (socklen_t) sizeof(struct sctp_event_subscribe); ret = getsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS, &event, &sockopt_len); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; event.sctp_authentication_event = 1; ret = setsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS, &event, sizeof(struct sctp_event_subscribe)); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; #endif #endif @@ -770,9 +780,14 @@ BIO_new_dgram_sctp(int fd, int close_flag) * larger than the max record size of 2^14 + 2048 + 13 */ ret = setsockopt(fd, IPPROTO_SCTP, SCTP_PARTIAL_DELIVERY_POINT, &optval, sizeof(optval)); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; return (bio); + +err: + BIO_vfree(bio); + return (NULL); } int @@ -929,16 +944,25 @@ dgram_sctp_read(BIO *b, char *out, int outl) event.se_type = SCTP_SENDER_DRY_EVENT; event.se_on = 0; i = setsockopt(b->num, IPPROTO_SCTP, SCTP_EVENT, &event, sizeof(struct sctp_event)); - OPENSSL_assert(i >= 0); + if (i < 0) { + ret = i; + break; + } #else eventsize = sizeof(struct sctp_event_subscribe); i = getsockopt(b->num, IPPROTO_SCTP, SCTP_EVENTS, &event, &eventsize); - OPENSSL_assert(i >= 0); + if (i < 0) { + ret = i; + break; + } event.sctp_sender_dry_event = 0; i = setsockopt(b->num, IPPROTO_SCTP, SCTP_EVENTS, &event, sizeof(struct sctp_event_subscribe)); - OPENSSL_assert(i >= 0); + if (i < 0) { + ret = i; + break; + } #endif } @@ -969,8 +993,8 @@ dgram_sctp_read(BIO *b, char *out, int outl) */ optlen = (socklen_t) sizeof(int); ret = getsockopt(b->num, SOL_SOCKET, SO_RCVBUF, &optval, &optlen); - OPENSSL_assert(ret >= 0); - OPENSSL_assert(optval >= 18445); + if (ret >= 0) + OPENSSL_assert(optval >= 18445); /* Test if SCTP doesn't partially deliver below * max record size (2^14 + 2048 + 13) @@ -978,8 +1002,8 @@ dgram_sctp_read(BIO *b, char *out, int outl) optlen = (socklen_t) sizeof(int); ret = getsockopt(b->num, IPPROTO_SCTP, SCTP_PARTIAL_DELIVERY_POINT, &optval, &optlen); - OPENSSL_assert(ret >= 0); - OPENSSL_assert(optval >= 18445); + if (ret >= 0) + OPENSSL_assert(optval >= 18445); /* Partially delivered notification??? Probably a bug.... */ OPENSSL_assert(!(msg.msg_flags & MSG_NOTIFICATION)); @@ -1008,16 +1032,21 @@ dgram_sctp_read(BIO *b, char *out, int outl) optlen = (socklen_t)(sizeof(sctp_assoc_t) + 256 * sizeof(uint8_t)); authchunks = calloc(1, optlen); + if (authchunks == NULL) { + BIOerr(BIO_F_DGRAM_SCTP_READ, + ERR_R_MALLOC_ERROR); + return (-1); + } ii = getsockopt(b->num, IPPROTO_SCTP, SCTP_PEER_AUTH_CHUNKS, authchunks, &optlen); - OPENSSL_assert(ii >= 0); - - for (p = (unsigned char*) authchunks->gauth_chunks; - p < (unsigned char*) authchunks + optlen; - p += sizeof(uint8_t)) { - if (*p == OPENSSL_SCTP_DATA_CHUNK_TYPE) - auth_data = 1; - if (*p == OPENSSL_SCTP_FORWARD_CUM_TSN_CHUNK_TYPE) - auth_forward = 1; + if (ii >= 0) { + for (p = (unsigned char*) authchunks->gauth_chunks; + p < (unsigned char*) authchunks + optlen; + p += sizeof(uint8_t)) { + if (*p == OPENSSL_SCTP_DATA_CHUNK_TYPE) + auth_data = 1; + if (*p == OPENSSL_SCTP_FORWARD_CUM_TSN_CHUNK_TYPE) + auth_forward = 1; + } } free(authchunks); diff --git a/lib/libssl/src/crypto/bio/bss_dgram.c b/lib/libssl/src/crypto/bio/bss_dgram.c index 2e17dc9e21d..c6b552eb322 100644 --- a/lib/libssl/src/crypto/bio/bss_dgram.c +++ b/lib/libssl/src/crypto/bio/bss_dgram.c @@ -1,4 +1,4 @@ -/* $OpenBSD: bss_dgram.c,v 1.37 2014/11/26 05:41:44 bcook Exp $ */ +/* $OpenBSD: bss_dgram.c,v 1.38 2015/01/03 18:07:29 doug Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -719,18 +719,25 @@ BIO_new_dgram_sctp(int fd, int close_flag) /* Activate SCTP-AUTH for DATA and FORWARD-TSN chunks */ auth.sauth_chunk = OPENSSL_SCTP_DATA_CHUNK_TYPE; ret = setsockopt(fd, IPPROTO_SCTP, SCTP_AUTH_CHUNK, &auth, sizeof(struct sctp_authchunk)); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; auth.sauth_chunk = OPENSSL_SCTP_FORWARD_CUM_TSN_CHUNK_TYPE; ret = setsockopt(fd, IPPROTO_SCTP, SCTP_AUTH_CHUNK, &auth, sizeof(struct sctp_authchunk)); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; /* Test if activation was successful. When using accept(), * SCTP-AUTH has to be activated for the listening socket * already, otherwise the connected socket won't use it. */ sockopt_len = (socklen_t)(sizeof(sctp_assoc_t) + 256 * sizeof(uint8_t)); authchunks = calloc(1, sockopt_len); + if (authchunks == NULL) + goto err; ret = getsockopt(fd, IPPROTO_SCTP, SCTP_LOCAL_AUTH_CHUNKS, authchunks, &sockopt_len); - OPENSSL_assert(ret >= 0); + if (ret < 0) { + free(authchunks); + goto err; + } for (p = (unsigned char*) authchunks->gauth_chunks; p < (unsigned char*) authchunks + sockopt_len; @@ -753,16 +760,19 @@ BIO_new_dgram_sctp(int fd, int close_flag) event.se_type = SCTP_AUTHENTICATION_EVENT; event.se_on = 1; ret = setsockopt(fd, IPPROTO_SCTP, SCTP_EVENT, &event, sizeof(struct sctp_event)); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; #else sockopt_len = (socklen_t) sizeof(struct sctp_event_subscribe); ret = getsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS, &event, &sockopt_len); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; event.sctp_authentication_event = 1; ret = setsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS, &event, sizeof(struct sctp_event_subscribe)); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; #endif #endif @@ -770,9 +780,14 @@ BIO_new_dgram_sctp(int fd, int close_flag) * larger than the max record size of 2^14 + 2048 + 13 */ ret = setsockopt(fd, IPPROTO_SCTP, SCTP_PARTIAL_DELIVERY_POINT, &optval, sizeof(optval)); - OPENSSL_assert(ret >= 0); + if (ret < 0) + goto err; return (bio); + +err: + BIO_vfree(bio); + return (NULL); } int @@ -929,16 +944,25 @@ dgram_sctp_read(BIO *b, char *out, int outl) event.se_type = SCTP_SENDER_DRY_EVENT; event.se_on = 0; i = setsockopt(b->num, IPPROTO_SCTP, SCTP_EVENT, &event, sizeof(struct sctp_event)); - OPENSSL_assert(i >= 0); + if (i < 0) { + ret = i; + break; + } #else eventsize = sizeof(struct sctp_event_subscribe); i = getsockopt(b->num, IPPROTO_SCTP, SCTP_EVENTS, &event, &eventsize); - OPENSSL_assert(i >= 0); + if (i < 0) { + ret = i; + break; + } event.sctp_sender_dry_event = 0; i = setsockopt(b->num, IPPROTO_SCTP, SCTP_EVENTS, &event, sizeof(struct sctp_event_subscribe)); - OPENSSL_assert(i >= 0); + if (i < 0) { + ret = i; + break; + } #endif } @@ -969,8 +993,8 @@ dgram_sctp_read(BIO *b, char *out, int outl) */ optlen = (socklen_t) sizeof(int); ret = getsockopt(b->num, SOL_SOCKET, SO_RCVBUF, &optval, &optlen); - OPENSSL_assert(ret >= 0); - OPENSSL_assert(optval >= 18445); + if (ret >= 0) + OPENSSL_assert(optval >= 18445); /* Test if SCTP doesn't partially deliver below * max record size (2^14 + 2048 + 13) @@ -978,8 +1002,8 @@ dgram_sctp_read(BIO *b, char *out, int outl) optlen = (socklen_t) sizeof(int); ret = getsockopt(b->num, IPPROTO_SCTP, SCTP_PARTIAL_DELIVERY_POINT, &optval, &optlen); - OPENSSL_assert(ret >= 0); - OPENSSL_assert(optval >= 18445); + if (ret >= 0) + OPENSSL_assert(optval >= 18445); /* Partially delivered notification??? Probably a bug.... */ OPENSSL_assert(!(msg.msg_flags & MSG_NOTIFICATION)); @@ -1008,16 +1032,21 @@ dgram_sctp_read(BIO *b, char *out, int outl) optlen = (socklen_t)(sizeof(sctp_assoc_t) + 256 * sizeof(uint8_t)); authchunks = calloc(1, optlen); + if (authchunks == NULL) { + BIOerr(BIO_F_DGRAM_SCTP_READ, + ERR_R_MALLOC_ERROR); + return (-1); + } ii = getsockopt(b->num, IPPROTO_SCTP, SCTP_PEER_AUTH_CHUNKS, authchunks, &optlen); - OPENSSL_assert(ii >= 0); - - for (p = (unsigned char*) authchunks->gauth_chunks; - p < (unsigned char*) authchunks + optlen; - p += sizeof(uint8_t)) { - if (*p == OPENSSL_SCTP_DATA_CHUNK_TYPE) - auth_data = 1; - if (*p == OPENSSL_SCTP_FORWARD_CUM_TSN_CHUNK_TYPE) - auth_forward = 1; + if (ii >= 0) { + for (p = (unsigned char*) authchunks->gauth_chunks; + p < (unsigned char*) authchunks + optlen; + p += sizeof(uint8_t)) { + if (*p == OPENSSL_SCTP_DATA_CHUNK_TYPE) + auth_data = 1; + if (*p == OPENSSL_SCTP_FORWARD_CUM_TSN_CHUNK_TYPE) + auth_forward = 1; + } } free(authchunks); -- 2.20.1