CVE-2017-5337: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: Multiple heap-based buffer overflows in the read_attribute function in GnuTLS before 3.3.26 and 3.5.x before 3.5.8 allow remote attackers to have unspecified impact via a crafted OpenPGP certificate.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 9.8
Published: 2017-03-24T15:59Z
Repository: https://gitlab.com/gnutls/gnutls
Language: C
Files changed: lib/opencdk/read-packet.c
Commit date: 2017-01-04 15:24:57 +0100
Commit message: opencdk: read_attribute: added more precise checks when reading stream

That addresses heap read overflows found using oss-fuzz:
  https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=338
  https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=346

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

Diff: diff --git a/lib/opencdk/read-packet.c b/lib/opencdk/read-packet.c
index cdb47007a6..b999e1c000 100644
--- a/lib/opencdk/read-packet.c
+++ b/lib/opencdk/read-packet.c
@@ -480,50 +480,68 @@ read_attribute(cdk_stream_t inp, size_t pktlen, cdk_pkt_userid_t attr,
 	buf = cdk_calloc(1, pktlen);
 	if (!buf)
 		return CDK_Out_Of_Core;
 	rc = stream_read(inp, buf, pktlen, &nread);
 	if (rc) {
-		cdk_free(buf);
-		return CDK_Inv_Packet;
+		gnutls_assert();
+		rc = CDK_Inv_Packet;
+		goto error;
 	}
+
 	p = buf;
 	len = *p++;
 	pktlen--;
+
 	if (len == 255) {
+		if (pktlen < 4) {
+			gnutls_assert();
+			rc = CDK_Inv_Packet;
+			goto error;
+		}
+
 		len = _cdk_buftou32(p);
 		p += 4;
 		pktlen -= 4;
 	} else if (len >= 192) {
 		if (pktlen < 2) {
-			cdk_free(buf);
-			return CDK_Inv_Packet;
+			gnutls_assert();
+			rc = CDK_Inv_Packet;
+			goto error;
 		}
+
 		len = ((len - 192) << 8) + *p + 192;
 		p++;
 		pktlen--;
 	}
 
-	if (*p != 1) {		/* Currently only 1, meaning an image, is defined. */
-		cdk_free(buf);
-		return CDK_Inv_Packet;
+	if (!len || *p != 1) {		/* Currently only 1, meaning an image, is defined. */
+		rc = CDK_Inv_Packet;
+		goto error;
 	}
+
 	p++;
 	len--;
 
 	if (len >= pktlen) {
-		cdk_free(buf);
-		return CDK_Inv_Packet;
+		rc = CDK_Inv_Packet;
+		goto error;
 	}
+
 	attr->attrib_img = cdk_calloc(1, len);
 	if (!attr->attrib_img) {
-		cdk_free(buf);
-		return CDK_Out_Of_Core;
+		rc = CDK_Out_Of_Core;
+		goto error;
 	}
+
 	attr->attrib_len = len;
 	memcpy(attr->attrib_img, p, len);
 	cdk_free(buf);
 	return rc;
+
+ error:
+	cdk_free(buf);
+	return rc;
 }
 
 
 static cdk_error_t
 read_user_id(cdk_stream_t inp, size_t pktlen, cdk_pkt_userid_t user_id)

Vulnerable code: 		cdk_free(buf);
		return CDK_Inv_Packet;
			cdk_free(buf);
			return CDK_Inv_Packet;
	if (*p != 1) {		/* Currently only 1, meaning an image, is defined. */
		cdk_free(buf);
		return CDK_Inv_Packet;
		cdk_free(buf);
		return CDK_Inv_Packet;
		cdk_free(buf);
		return CDK_Out_Of_Core;
Fixed code: 		gnutls_assert();
		rc = CDK_Inv_Packet;
		goto error;


		if (pktlen < 4) {
			gnutls_assert();
			rc = CDK_Inv_Packet;
			goto error;
		}

			gnutls_assert();
			rc = CDK_Inv_Packet;
			goto error;

	if (!len || *p != 1) {		/* Currently only 1, meaning an image, is defined. */
		rc = CDK_Inv_Packet;
		goto error;

		rc = CDK_Inv_Packet;
		goto error;

		rc = CDK_Out_Of_Core;
		goto error;


 error:
	cdk_free(buf);
	return rc;