From d1ca61581d41ca664ca2be81b4cc385710e166f6 Mon Sep 17 00:00:00 2001 From: doug Date: Sat, 25 Apr 2015 15:28:47 +0000 Subject: [PATCH] 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@ --- lib/libssl/bs_cbs.c | 11 ++++++++--- lib/libssl/src/ssl/bs_cbs.c | 11 ++++++++--- regress/lib/libssl/bytestring/bytestringtest.c | 6 ++++-- 3 files changed, 20 insertions(+), 8 deletions(-) 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 -- 2.20.1