Add size_t to int checks for SSL functions.
authordoug <doug@openbsd.org>
Wed, 17 Dec 2014 17:51:33 +0000 (17:51 +0000)
committerdoug <doug@openbsd.org>
Wed, 17 Dec 2014 17:51:33 +0000 (17:51 +0000)
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
lib/libtls/tls_internal.h
lib/libtls/tls_verify.c

index 6dae066..0b9f125 100644 (file)
@@ -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 <jsing@openbsd.org>
  *
@@ -18,6 +18,7 @@
 #include <sys/socket.h>
 
 #include <errno.h>
+#include <limits.h>
 #include <stdlib.h>
 #include <unistd.h>
 
@@ -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;
index bfd7146..4b25057 100644 (file)
@@ -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 <jca@openbsd.org>
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
@@ -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 */
index 697432c..4341802 100644 (file)
@@ -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 <jca@openbsd.org>
  *
@@ -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);