Extend the input types for CBB_add_*() to help catch bugs.

While the previous types were correct, they can silently accept bad data
via truncation or signed conversion.  We now take size_t as input for
CBB_add_u*() and do a range check.

discussed with deraadt@
input + ok jsing@ miod@

diff --git a/lib/libssl/bs_cbb.c b/lib/libssl/bs_cbb.c
index e86bb92..4411417 100644 (file)
--- a/lib/libssl/bs_cbb.c
+++ b/lib/libssl/bs_cbb.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: bs_cbb.c,v 1.11 2015/06/13 09:24:12 doug Exp $        */
+/*     $OpenBSD: bs_cbb.c,v 1.12 2015/06/18 23:25:07 doug Exp $        */
 /*
  * Copyright (c) 2014, Google Inc.
  *
@@ -304,8 +304,11 @@ CBB_add_u24_length_prefixed(CBB *cbb, CBB *out_contents)
 }
 
 int
-CBB_add_asn1(CBB *cbb, CBB *out_contents, uint8_t tag)
+CBB_add_asn1(CBB *cbb, CBB *out_contents, unsigned int tag)
 {
+       if (tag > UINT8_MAX)
+               return 0;
+
        /* Long form identifier octets are not supported. */
        if ((tag & 0x1f) == 0x1f)
                return 0;
@@ -353,21 +356,30 @@ CBB_add_space(CBB *cbb, uint8_t **out_data, size_t len)
 }
 
 int
-CBB_add_u8(CBB *cbb, uint8_t value)
+CBB_add_u8(CBB *cbb, size_t value)
 {
-       return cbb_add_u(cbb, value, 1);
+       if (value > UINT8_MAX)
+               return 0;
+
+       return cbb_add_u(cbb, (uint32_t)value, 1);
 }
 
 int
-CBB_add_u16(CBB *cbb, uint16_t value)
+CBB_add_u16(CBB *cbb, size_t value)
 {
-       return cbb_add_u(cbb, value, 2);
+       if (value > UINT16_MAX)
+               return 0;
+
+       return cbb_add_u(cbb, (uint32_t)value, 2);
 }
 
 int
-CBB_add_u24(CBB *cbb, uint32_t value)
+CBB_add_u24(CBB *cbb, size_t value)
 {
-       return cbb_add_u(cbb, value, 3);
+       if (value > 0xffffffUL)
+               return 0;
+
+       return cbb_add_u(cbb, (uint32_t)value, 3);
 }
 
 int

diff --git a/lib/libssl/bytestring.h b/lib/libssl/bytestring.h
index e831706..4c9d4d8 100644 (file)
--- a/lib/libssl/bytestring.h
+++ b/lib/libssl/bytestring.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: bytestring.h,v 1.12 2015/06/17 07:25:56 doug Exp $    */
+/*     $OpenBSD: bytestring.h,v 1.13 2015/06/18 23:25:07 doug Exp $    */
 /*
  * Copyright (c) 2014, Google Inc.
  *
@@ -423,7 +423,7 @@ int CBB_add_u24_length_prefixed(CBB *cbb, CBB *out_contents);
  * single octet identifiers are supported. It returns one on success or zero
  * on error.
  */
-int CBB_add_asn1(CBB *cbb, CBB *out_contents, uint8_t tag);
+int CBB_add_asn1(CBB *cbb, CBB *out_contents, unsigned int tag);
 
 /*
  * CBB_add_bytes appends |len| bytes from |data| to |cbb|. It returns one on
@@ -443,19 +443,19 @@ int CBB_add_space(CBB *cbb, uint8_t **out_data, size_t len);
  * CBB_add_u8 appends an 8-bit number from |value| to |cbb|. It returns one on
  * success and zero otherwise.
  */
-int CBB_add_u8(CBB *cbb, uint8_t value);
+int CBB_add_u8(CBB *cbb, size_t value);
 
 /*
  * CBB_add_u8 appends a 16-bit, big-endian number from |value| to |cbb|. It
  * returns one on success and zero otherwise.
  */
-int CBB_add_u16(CBB *cbb, uint16_t value);
+int CBB_add_u16(CBB *cbb, size_t value);
 
 /*
  * CBB_add_u24 appends a 24-bit, big-endian number from |value| to |cbb|. It
  * returns one on success and zero otherwise.
  */
-int CBB_add_u24(CBB *cbb, uint32_t value);
+int CBB_add_u24(CBB *cbb, size_t value);
 
 /*
  * CBB_add_asn1_uint64 writes an ASN.1 INTEGER into |cbb| using |CBB_add_asn1|

diff --git a/lib/libssl/src/ssl/bs_cbb.c b/lib/libssl/src/ssl/bs_cbb.c
index e86bb92..4411417 100644 (file)
--- a/lib/libssl/src/ssl/bs_cbb.c
+++ b/lib/libssl/src/ssl/bs_cbb.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: bs_cbb.c,v 1.11 2015/06/13 09:24:12 doug Exp $        */
+/*     $OpenBSD: bs_cbb.c,v 1.12 2015/06/18 23:25:07 doug Exp $        */
 /*
  * Copyright (c) 2014, Google Inc.
  *
@@ -304,8 +304,11 @@ CBB_add_u24_length_prefixed(CBB *cbb, CBB *out_contents)
 }
 
 int
-CBB_add_asn1(CBB *cbb, CBB *out_contents, uint8_t tag)
+CBB_add_asn1(CBB *cbb, CBB *out_contents, unsigned int tag)
 {
+       if (tag > UINT8_MAX)
+               return 0;
+
        /* Long form identifier octets are not supported. */
        if ((tag & 0x1f) == 0x1f)
                return 0;
@@ -353,21 +356,30 @@ CBB_add_space(CBB *cbb, uint8_t **out_data, size_t len)
 }
 
 int
-CBB_add_u8(CBB *cbb, uint8_t value)
+CBB_add_u8(CBB *cbb, size_t value)
 {
-       return cbb_add_u(cbb, value, 1);
+       if (value > UINT8_MAX)
+               return 0;
+
+       return cbb_add_u(cbb, (uint32_t)value, 1);
 }
 
 int
-CBB_add_u16(CBB *cbb, uint16_t value)
+CBB_add_u16(CBB *cbb, size_t value)
 {
-       return cbb_add_u(cbb, value, 2);
+       if (value > UINT16_MAX)
+               return 0;
+
+       return cbb_add_u(cbb, (uint32_t)value, 2);
 }
 
 int
-CBB_add_u24(CBB *cbb, uint32_t value)
+CBB_add_u24(CBB *cbb, size_t value)
 {
-       return cbb_add_u(cbb, value, 3);
+       if (value > 0xffffffUL)
+               return 0;
+
+       return cbb_add_u(cbb, (uint32_t)value, 3);
 }
 
 int

diff --git a/lib/libssl/src/ssl/bytestring.h b/lib/libssl/src/ssl/bytestring.h
index e831706..4c9d4d8 100644 (file)
--- a/lib/libssl/src/ssl/bytestring.h
+++ b/lib/libssl/src/ssl/bytestring.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: bytestring.h,v 1.12 2015/06/17 07:25:56 doug Exp $    */
+/*     $OpenBSD: bytestring.h,v 1.13 2015/06/18 23:25:07 doug Exp $    */
 /*
  * Copyright (c) 2014, Google Inc.
  *
@@ -423,7 +423,7 @@ int CBB_add_u24_length_prefixed(CBB *cbb, CBB *out_contents);
  * single octet identifiers are supported. It returns one on success or zero
  * on error.
  */
-int CBB_add_asn1(CBB *cbb, CBB *out_contents, uint8_t tag);
+int CBB_add_asn1(CBB *cbb, CBB *out_contents, unsigned int tag);
 
 /*
  * CBB_add_bytes appends |len| bytes from |data| to |cbb|. It returns one on
@@ -443,19 +443,19 @@ int CBB_add_space(CBB *cbb, uint8_t **out_data, size_t len);
  * CBB_add_u8 appends an 8-bit number from |value| to |cbb|. It returns one on
  * success and zero otherwise.
  */
-int CBB_add_u8(CBB *cbb, uint8_t value);
+int CBB_add_u8(CBB *cbb, size_t value);
 
 /*
  * CBB_add_u8 appends a 16-bit, big-endian number from |value| to |cbb|. It
  * returns one on success and zero otherwise.
  */
-int CBB_add_u16(CBB *cbb, uint16_t value);
+int CBB_add_u16(CBB *cbb, size_t value);
 
 /*
  * CBB_add_u24 appends a 24-bit, big-endian number from |value| to |cbb|. It
  * returns one on success and zero otherwise.
  */
-int CBB_add_u24(CBB *cbb, uint32_t value);
+int CBB_add_u24(CBB *cbb, size_t value);
 
 /*
  * CBB_add_asn1_uint64 writes an ASN.1 INTEGER into |cbb| using |CBB_add_asn1|