From 9d4b5ca74c88ec79c48a23a2626fb296b06d1e70 Mon Sep 17 00:00:00 2001
From: doug <doug@openbsd.org>
Date: Thu, 18 Jun 2015 23:25:07 +0000
Subject: [PATCH] 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@
---
 lib/libssl/bs_cbb.c             | 28 ++++++++++++++++++++--------
 lib/libssl/bytestring.h         | 10 +++++-----
 lib/libssl/src/ssl/bs_cbb.c     | 28 ++++++++++++++++++++--------
 lib/libssl/src/ssl/bytestring.h | 10 +++++-----
 4 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/lib/libssl/bs_cbb.c b/lib/libssl/bs_cbb.c
index e86bb926ab8..441141734bf 100644
--- 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 e831706b28d..4c9d4d88847 100644
--- 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 e86bb926ab8..441141734bf 100644
--- 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 e831706b28d..4c9d4d88847 100644
--- 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|
-- 
2.20.1