linux-kernel - [PATCH 3.12 74/76] KEYS: Fix ASN.1 indefinite length object parsing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <012d81589afb85807b7ebe72f3505d90f34aa265.1463648873.git.jslaby@suse.cz>
Date:	Thu, 19 May 2016 11:08:36 +0200
From:	Jiri Slaby <jslaby@...e.cz>
To:	stable@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, David Howells <dhowells@...hat.com>,
	Jiri Slaby <jslaby@...e.cz>
Subject: [PATCH 3.12 74/76] KEYS: Fix ASN.1 indefinite length object parsing

From: David Howells <dhowells@...hat.com>

3.12-stable review patch.  If anyone has any objections, please let me know.

===============

commit 23c8a812dc3c621009e4f0e5342aa4e2ede1ceaa upstream.

This fixes CVE-2016-0758.

In the ASN.1 decoder, when the length field of an ASN.1 value is extracted,
it isn't validated against the remaining amount of data before being added
to the cursor.  With a sufficiently large size indicated, the check:

	datalen - dp < 2

may then fail due to integer overflow.

Fix this by checking the length indicated against the amount of remaining
data in both places a definite length is determined.

Whilst we're at it, make the following changes:

 (1) Check the maximum size of extended length does not exceed the capacity
     of the variable it's being stored in (len) rather than the type that
     variable is assumed to be (size_t).

 (2) Compare the EOC tag to the symbolic constant ASN1_EOC rather than the
     integer 0.

 (3) To reduce confusion, move the initialisation of len outside of:

	for (len = 0; n > 0; n--) {

     since it doesn't have anything to do with the loop counter n.

Signed-off-by: David Howells <dhowells@...hat.com>
Reviewed-by: Mimi Zohar <zohar@...ux.vnet.ibm.com>
Acked-by: David Woodhouse <David.Woodhouse@...el.com>
Acked-by: Peter Jones <pjones@...hat.com>
Signed-off-by: Jiri Slaby <jslaby@...e.cz>
---
 lib/asn1_decoder.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c
index 3787d02e2c49..b1c885297113 100644
--- a/lib/asn1_decoder.c
+++ b/lib/asn1_decoder.c
@@ -69,7 +69,7 @@ next_tag:
 
 	/* Extract a tag from the data */
 	tag = data[dp++];
-	if (tag == 0) {
+	if (tag == ASN1_EOC) {
 		/* It appears to be an EOC. */
 		if (data[dp++] != 0)
 			goto invalid_eoc;
@@ -91,10 +91,8 @@ next_tag:
 
 	/* Extract the length */
 	len = data[dp++];
-	if (len <= 0x7f) {
-		dp += len;
-		goto next_tag;
-	}
+	if (len <= 0x7f)
+		goto check_length;
 
 	if (unlikely(len == ASN1_INDEFINITE_LENGTH)) {
 		/* Indefinite length */
@@ -105,14 +103,18 @@ next_tag:
 	}
 
 	n = len - 0x80;
-	if (unlikely(n > sizeof(size_t) - 1))
+	if (unlikely(n > sizeof(len) - 1))
 		goto length_too_long;
 	if (unlikely(n > datalen - dp))
 		goto data_overrun_error;
-	for (len = 0; n > 0; n--) {
+	len = 0;
+	for (; n > 0; n--) {
 		len <<= 8;
 		len |= data[dp++];
 	}
+check_length:
+	if (len > datalen - dp)
+		goto data_overrun_error;
 	dp += len;
 	goto next_tag;
 
-- 
2.8.2