From ca4bb8fd7a5a26fd8735668aa2353f221a0d0fbc Mon Sep 17 00:00:00 2001
From: beck <>
Date: Sat, 23 Mar 2019 18:48:15 +0000
Subject: Add range checks to varios ASN1_INTEGER functions to ensure the sizes
 used remain a positive integer. Should address issue 13799 from oss-fuzz ok
 tb@ jsing@

---
 src/lib/libcrypto/asn1/a_int.c | 56 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 3 deletions(-)

(limited to 'src/lib/libcrypto/asn1/a_int.c')

diff --git a/src/lib/libcrypto/asn1/a_int.c b/src/lib/libcrypto/asn1/a_int.c
index 95d0f6dbb2..218d0b607b 100644
--- a/src/lib/libcrypto/asn1/a_int.c
+++ b/src/lib/libcrypto/asn1/a_int.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: a_int.c,v 1.31 2017/01/29 17:49:22 beck Exp $ */
+/* $OpenBSD: a_int.c,v 1.32 2019/03/23 18:48:14 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -59,13 +59,24 @@
 #include <stdio.h>
 #include <string.h>
 
+#include <sys/limits.h>
+
 #include <openssl/asn1.h>
 #include <openssl/bn.h>
 #include <openssl/err.h>
 
+static int
+ASN1_INTEGER_valid(const ASN1_INTEGER *a)
+{
+	return (a != NULL && a->length >= 0);
+}
+
 ASN1_INTEGER *
 ASN1_INTEGER_dup(const ASN1_INTEGER *x)
 {
+	if (!ASN1_INTEGER_valid(x))
+		return NULL;
+
 	return ASN1_STRING_dup(x);
 }
 
@@ -123,8 +134,9 @@ i2c_ASN1_INTEGER(ASN1_INTEGER *a, unsigned char **pp)
 	int pad = 0, ret, i, neg;
 	unsigned char *p, *n, pb = 0;
 
-	if (a == NULL)
-		return (0);
+	if (!ASN1_INTEGER_valid(a))
+		return 0;
+
 	neg = a->type & V_ASN1_NEG;
 	if (a->length == 0)
 		ret = 1;
@@ -201,11 +213,24 @@ c2i_ASN1_INTEGER(ASN1_INTEGER **a, const unsigned char **pp, long len)
 	} else
 		ret = (*a);
 
+	if (!ASN1_INTEGER_valid(ret)) {
+		/*
+		 * XXX using i for an alert is confusing,
+		 * we should call this al
+		 */
+		i = ERR_R_ASN1_LENGTH_MISMATCH;
+		goto err;
+	}
+
 	p = *pp;
 	pend = p + len;
 
 	/* We must malloc stuff, even for 0 bytes otherwise it
 	 * signifies a missing NULL parameter. */
+	if (len < 0 || len > INT_MAX) {
+		i = ERR_R_ASN1_LENGTH_MISMATCH;
+		goto err;
+	}
 	s = malloc(len + 1);
 	if (s == NULL) {
 		i = ERR_R_MALLOC_FAILURE;
@@ -294,6 +319,11 @@ d2i_ASN1_UINTEGER(ASN1_INTEGER **a, const unsigned char **pp, long length)
 	} else
 		ret = (*a);
 
+	if (!ASN1_INTEGER_valid(ret)) {
+		i = ERR_R_ASN1_LENGTH_MISMATCH;
+		goto err;
+	}
+
 	p = *pp;
 	inf = ASN1_get_object(&p, &len, &tag, &xclass, length);
 	if (inf & 0x80) {
@@ -308,6 +338,10 @@ d2i_ASN1_UINTEGER(ASN1_INTEGER **a, const unsigned char **pp, long length)
 
 	/* We must malloc stuff, even for 0 bytes otherwise it
 	 * signifies a missing NULL parameter. */
+	if (len < 0 || len > INT_MAX) {
+		i = ERR_R_ASN1_LENGTH_MISMATCH;
+		goto err;
+	}
 	s = malloc(len + 1);
 	if (s == NULL) {
 		i = ERR_R_MALLOC_FAILURE;
@@ -375,6 +409,12 @@ ASN1_INTEGER_set(ASN1_INTEGER *a, long v)
 	return (1);
 }
 
+/*
+ * XXX this particular API is a gibbering eidrich horror that makes it
+ * impossible to determine valid return cases from errors.. "a bit
+ * ugly" is preserved for posterity, unfortunately this is probably
+ * unfixable without changing public API
+ */
 long
 ASN1_INTEGER_get(const ASN1_INTEGER *a)
 {
@@ -389,6 +429,9 @@ ASN1_INTEGER_get(const ASN1_INTEGER *a)
 	else if (i != V_ASN1_INTEGER)
 		return -1;
 
+	if (!ASN1_INTEGER_valid(a))
+		return -1; /* XXX best effort */
+
 	if (a->length > (int)sizeof(long)) {
 		/* hmm... a bit ugly, return all ones */
 		return -1;
@@ -419,6 +462,10 @@ BN_to_ASN1_INTEGER(const BIGNUM *bn, ASN1_INTEGER *ai)
 		ASN1error(ERR_R_NESTED_ASN1_ERROR);
 		goto err;
 	}
+
+	if (!ASN1_INTEGER_valid(ret))
+		goto err;
+
 	if (BN_is_negative(bn))
 		ret->type = V_ASN1_NEG_INTEGER;
 	else
@@ -453,6 +500,9 @@ ASN1_INTEGER_to_BN(const ASN1_INTEGER *ai, BIGNUM *bn)
 {
 	BIGNUM *ret;
 
+	if (!ASN1_INTEGER_valid(ai))
+		return (NULL);
+
 	if ((ret = BN_bin2bn(ai->data, ai->length, bn)) == NULL)
 		ASN1error(ASN1_R_BN_LIB);
 	else if (ai->type == V_ASN1_NEG_INTEGER)
-- 
cgit v1.2.3-55-g6feb