From: doug <doug@openbsd.org>
Date: Sat, 25 Apr 2015 15:28:47 +0000 (+0000)
Subject: Check for invalid leading zeros in CBS_get_asn1_uint64.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=d1ca61581d41ca664ca2be81b4cc385710e166f6;p=openbsd

Check for invalid leading zeros in CBS_get_asn1_uint64.

ASN.1 integers cannot have all zeros or all ones for the first 9 bits.
This rule ensures the numbers are encoded with the smallest number of
content octets (see ITU-T Rec X.690 section 8.3.2).

Based on BoringSSL commit 5933723b7b592e9914f703d630b596e140c93e16

ok deraadt@ jsing@
---

diff --git a/lib/libssl/bs_cbs.c b/lib/libssl/bs_cbs.c
index c3d3a8abf2a..d7c0977cf3f 100644
--- a/lib/libssl/bs_cbs.c
+++ b/lib/libssl/bs_cbs.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bs_cbs.c,v 1.2 2015/02/06 22:22:33 doug Exp $	*/
+/*	$OpenBSD: bs_cbs.c,v 1.3 2015/04/25 15:28:47 doug Exp $	*/
 /*
  * Copyright (c) 2014, Google Inc.
  *
@@ -317,6 +317,7 @@ CBS_peek_asn1_tag(const CBS *cbs, unsigned tag_value)
 	return CBS_data(cbs)[0] == tag_value;
 }
 
+/* Encoding details are in ASN.1: X.690 section 8.3 */
 int
 CBS_get_asn1_uint64(CBS *cbs, uint64_t *out)
 {
@@ -332,11 +333,15 @@ CBS_get_asn1_uint64(CBS *cbs, uint64_t *out)
 	len = CBS_len(&bytes);
 
 	if (len == 0)
-		/* An INTEGER is encoded with at least one octet. */
+		/* An INTEGER is encoded with at least one content octet. */
 		return 0;
 
 	if ((data[0] & 0x80) != 0)
-		/* negative number */
+		/* Negative number. */
+		return 0;
+
+	if (data[0] == 0 && len > 1 && (data[1] & 0x80) == 0)
+		/* Violates smallest encoding rule: excessive leading zeros. */
 		return 0;
 
 	for (i = 0; i < len; i++) {
diff --git a/lib/libssl/src/ssl/bs_cbs.c b/lib/libssl/src/ssl/bs_cbs.c
index c3d3a8abf2a..d7c0977cf3f 100644
--- a/lib/libssl/src/ssl/bs_cbs.c
+++ b/lib/libssl/src/ssl/bs_cbs.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bs_cbs.c,v 1.2 2015/02/06 22:22:33 doug Exp $	*/
+/*	$OpenBSD: bs_cbs.c,v 1.3 2015/04/25 15:28:47 doug Exp $	*/
 /*
  * Copyright (c) 2014, Google Inc.
  *
@@ -317,6 +317,7 @@ CBS_peek_asn1_tag(const CBS *cbs, unsigned tag_value)
 	return CBS_data(cbs)[0] == tag_value;
 }
 
+/* Encoding details are in ASN.1: X.690 section 8.3 */
 int
 CBS_get_asn1_uint64(CBS *cbs, uint64_t *out)
 {
@@ -332,11 +333,15 @@ CBS_get_asn1_uint64(CBS *cbs, uint64_t *out)
 	len = CBS_len(&bytes);
 
 	if (len == 0)
-		/* An INTEGER is encoded with at least one octet. */
+		/* An INTEGER is encoded with at least one content octet. */
 		return 0;
 
 	if ((data[0] & 0x80) != 0)
-		/* negative number */
+		/* Negative number. */
+		return 0;
+
+	if (data[0] == 0 && len > 1 && (data[1] & 0x80) == 0)
+		/* Violates smallest encoding rule: excessive leading zeros. */
 		return 0;
 
 	for (i = 0; i < len; i++) {
diff --git a/regress/lib/libssl/bytestring/bytestringtest.c b/regress/lib/libssl/bytestring/bytestringtest.c
index 82691511279..7ae9397a352 100644
--- a/regress/lib/libssl/bytestring/bytestringtest.c
+++ b/regress/lib/libssl/bytestring/bytestringtest.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bytestringtest.c,v 1.3 2015/02/16 06:48:17 doug Exp $	*/
+/*	$OpenBSD: bytestringtest.c,v 1.4 2015/04/25 15:28:47 doug Exp $	*/
 /*
  * Copyright (c) 2014, Google Inc.
  *
@@ -607,8 +607,10 @@ static const ASN1_INVALID_UINT64_TEST kAsn1InvalidUint64Tests[] = {
 	{"\x02\x00", 2},
 	/* Negative number. */
 	{"\x02\x01\x80", 3},
-	/* Overflow */
+	/* Overflow. */
 	{"\x02\x09\x01\x00\x00\x00\x00\x00\x00\x00\x00", 11},
+	/* Leading zeros. */
+	{"\x02\x02\x00\x01", 4},
 };
 
 static int