From c81cfcc3acd0162e621822063e38b68fea9e5902 Mon Sep 17 00:00:00 2001 From: doug Date: Wed, 17 Dec 2014 17:51:33 +0000 Subject: [PATCH] Add size_t to int checks for SSL functions. libtls accepts size_t for lengths but libssl accepts int. This verifies that the input does not exceed INT_MAX. It also avoids truncating size_t when comparing with int and adds printf-style attributes for tls_set_error(). with input from deraadt@ and tedu@ ok tedu@ --- lib/libtls/tls.c | 23 ++++++++++++++++++++++- lib/libtls/tls_internal.h | 6 ++++-- lib/libtls/tls_verify.c | 18 +++++++++++++----- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/lib/libtls/tls.c b/lib/libtls/tls.c index 6dae066922a..0b9f12511d6 100644 --- a/lib/libtls/tls.c +++ b/lib/libtls/tls.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls.c,v 1.3 2014/12/07 15:48:02 bcook Exp $ */ +/* $OpenBSD: tls.c,v 1.4 2014/12/17 17:51:33 doug Exp $ */ /* * Copyright (c) 2014 Joel Sing * @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -110,6 +111,11 @@ tls_configure_keypair(struct tls *ctx) BIO *bio = NULL; if (ctx->config->cert_mem != NULL) { + if (ctx->config->cert_len > INT_MAX) { + tls_set_error(ctx, "certificate too long"); + goto err; + } + if (SSL_CTX_use_certificate_chain(ctx->ssl_ctx, ctx->config->cert_mem, ctx->config->cert_len) != 1) { tls_set_error(ctx, "failed to load certificate"); @@ -118,6 +124,11 @@ tls_configure_keypair(struct tls *ctx) cert = NULL; } if (ctx->config->key_mem != NULL) { + if (ctx->config->key_len > INT_MAX) { + tls_set_error(ctx, "key too long"); + goto err; + } + if ((bio = BIO_new_mem_buf(ctx->config->key_mem, ctx->config->key_len)) == NULL) { tls_set_error(ctx, "failed to create buffer"); @@ -229,6 +240,11 @@ tls_read(struct tls *ctx, void *buf, size_t buflen, size_t *outlen) { int ret, ssl_err; + if (buflen > INT_MAX) { + tls_set_error(ctx, "buflen too long"); + return (-1); + } + ret = SSL_read(ctx->ssl_conn, buf, buflen); if (ret > 0) { *outlen = (size_t)ret; @@ -252,6 +268,11 @@ tls_write(struct tls *ctx, const void *buf, size_t buflen, size_t *outlen) { int ret, ssl_err; + if (buflen > INT_MAX) { + tls_set_error(ctx, "buflen too long"); + return (-1); + } + ret = SSL_write(ctx->ssl_conn, buf, buflen); if (ret > 0) { *outlen = (size_t)ret; diff --git a/lib/libtls/tls_internal.h b/lib/libtls/tls_internal.h index bfd7146d7d0..4b250574ef7 100644 --- a/lib/libtls/tls_internal.h +++ b/lib/libtls/tls_internal.h @@ -1,4 +1,4 @@ -/* $OpenBSD: tls_internal.h,v 1.4 2014/12/07 16:56:17 bcook Exp $ */ +/* $OpenBSD: tls_internal.h,v 1.5 2014/12/17 17:51:33 doug Exp $ */ /* * Copyright (c) 2014 Jeremie Courreges-Anglas * Copyright (c) 2014 Joel Sing @@ -67,6 +67,8 @@ int tls_configure_keypair(struct tls *ctx); int tls_configure_server(struct tls *ctx); int tls_configure_ssl(struct tls *ctx); int tls_host_port(const char *hostport, char **host, char **port); -int tls_set_error(struct tls *ctx, char *fmt, ...); +int tls_set_error(struct tls *ctx, char *fmt, ...) + __attribute__((__format__ (printf, 2, 3))) + __attribute__((__nonnull__ (2))); #endif /* HEADER_TLS_INTERNAL_H */ diff --git a/lib/libtls/tls_verify.c b/lib/libtls/tls_verify.c index 697432c429b..4341802b5ab 100644 --- a/lib/libtls/tls_verify.c +++ b/lib/libtls/tls_verify.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls_verify.c,v 1.5 2014/12/07 16:56:17 bcook Exp $ */ +/* $OpenBSD: tls_verify.c,v 1.6 2014/12/17 17:51:33 doug Exp $ */ /* * Copyright (c) 2014 Jeremie Courreges-Anglas * @@ -115,14 +115,14 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *host) if (type == GEN_DNS) { unsigned char *data; - int format; + int format, len; format = ASN1_STRING_type(altname->d.dNSName); if (format == V_ASN1_IA5STRING) { data = ASN1_STRING_data(altname->d.dNSName); + len = ASN1_STRING_length(altname->d.dNSName); - if (ASN1_STRING_length(altname->d.dNSName) != - (int)strlen(data)) { + if (len < 0 || len != strlen(data)) { tls_set_error(ctx, "error verifying host '%s': " "NUL byte in subjectAltName, " @@ -151,6 +151,14 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *host) datalen = ASN1_STRING_length(altname->d.iPAddress); data = ASN1_STRING_data(altname->d.iPAddress); + if (datalen < 0) { + tls_set_error(ctx, + "Unexpected negative length for an " + "IP address: %d", datalen); + rv = -2; + break; + } + if (datalen == addrlen && memcmp(data, &addrbuf, addrlen) == 0) { rv = 0; @@ -189,7 +197,7 @@ tls_check_common_name(struct tls *ctx, X509 *cert, const char *host) common_name_len + 1); /* NUL bytes in CN? */ - if (common_name_len != (int)strlen(common_name)) { + if (common_name_len != strlen(common_name)) { tls_set_error(ctx, "error verifying host '%s': " "NUL byte in Common Name field, " "probably a malicious certificate.", host); -- 2.20.1