ASN1_get_object should not accept large universal tags.chromium-2623

The high bits of the type get used for the V_ASN1_NEG bit, so when used with
ASN1_ANY/ASN1_TYPE, universal tags become ambiguous. This allows one to create
a negative zero, which should be impossible. Impose an upper bound on universal
tags accepted by crypto/asn1 and add a test.

BUG=590615

(Cherry-picked from fb2c6f8c8565e1e2d85c24408050c96521acbcdc.)

Change-Id: Ia988acf73fd11807869510a2b3825637a8d98853
Reviewed-on: https://boringssl-review.googlesource.com/7298
Reviewed-by: Adam Langley <agl@google.com>

6 files changed, 73 insertions, 0 deletions

diff --git a/crypto/asn1/CMakeLists.txt b/crypto/asn1/CMakeLists.txt
index 41e3122..df48e26 100644
--- a/crypto/asn1/CMakeLists.txt
+++ b/crypto/asn1/CMakeLists.txt
@@ -43,3 +43,14 @@ add_library(
   x_bignum.c
   x_long.c
 )
+
+add_executable(
+  asn1_test
+
+  asn1_test.cc
+
+  $<TARGET_OBJECTS:test_support>
+)
+
+target_link_libraries(asn1_test crypto)
+add_dependencies(all_tests asn1_test)
diff --git a/crypto/asn1/asn1_lib.c b/crypto/asn1/asn1_lib.c
index a109749..521c5bb 100644
--- a/crypto/asn1/asn1_lib.c
+++ b/crypto/asn1/asn1_lib.c
@@ -161,6 +161,11 @@ int ASN1_get_object(const unsigned char **pp, long *plength, int *ptag,
 		p++;
 		if (--max == 0) goto err;
 		}
+
+	/* To avoid ambiguity with V_ASN1_NEG, impose a limit on universal tags. */
+	if (xclass == V_ASN1_UNIVERSAL && tag > V_ASN1_MAX_UNIVERSAL)
+		goto err;
+
 	*ptag=tag;
 	*pclass=xclass;
 	if (!asn1_get_length(&p,&inf,plength,(int)max)) goto err;
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc
new file mode 100644
index 0000000..1044f7b
--- /dev/null
+++ b/crypto/asn1/asn1_test.cc
@@ -0,0 +1,51 @@
+/* Copyright (c) 2016, Google Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
+
+#include <stdio.h>
+
+#include <openssl/asn1.h>
+#include <openssl/crypto.h>
+#include <openssl/err.h>
+
+#include "../test/scoped_types.h"
+
+
+// kTag258 is an ASN.1 structure with a universal tag with number 258.
+static const uint8_t kTag258[] = {
+    0x1f, 0x82, 0x02, 0x01, 0x00,
+};
+
+static_assert(V_ASN1_NEG_INTEGER == 258,
+              "V_ASN1_NEG_INTEGER changed. Update kTag258 to collide with it.");
+
+bool TestLargeTags() {
+  const uint8_t *p = kTag258;
+  ScopedASN1_TYPE obj(d2i_ASN1_TYPE(NULL, &p, sizeof(kTag258)));
+  if (obj) {
+    fprintf(stderr, "Parsed value with illegal tag (type = %d).\n", obj->type);
+    return false;
+  }
+  return true;
+}
+
+int main() {
+  CRYPTO_library_init();
+
+  if (!TestLargeTags()) {
+    return 1;
+  }
+
+  printf("PASS\n");
+  return 0;
+}
diff --git a/crypto/test/scoped_types.h b/crypto/test/scoped_types.h
index 590f926..1c95211 100644
--- a/crypto/test/scoped_types.h
+++ b/crypto/test/scoped_types.h
@@ -21,6 +21,7 @@
 #include <memory>
 
 #include <openssl/aead.h>
+#include <openssl/asn1.h>
 #include <openssl/bio.h>
 #include <openssl/bn.h>
 #include <openssl/cmac.h>
@@ -95,6 +96,7 @@ class ScopedOpenSSLContext {
   T ctx_;
 };
 
+using ScopedASN1_TYPE = ScopedOpenSSLType<ASN1_TYPE, ASN1_TYPE_free>;
 using ScopedBIO = ScopedOpenSSLType<BIO, BIO_vfree>;
 using ScopedBIGNUM = ScopedOpenSSLType<BIGNUM, BN_free>;
 using ScopedBN_CTX = ScopedOpenSSLType<BN_CTX, BN_CTX_free>;
diff --git a/include/openssl/asn1.h b/include/openssl/asn1.h
index 08886d1..ac3fef0 100644
--- a/include/openssl/asn1.h
+++ b/include/openssl/asn1.h
@@ -85,6 +85,9 @@ extern "C" {
 #define V_ASN1_ANY			-4	/* used in ASN1 template code */
 
 #define V_ASN1_NEG			0x100	/* negative flag */
+/* No supported universal tags may exceed this value, to avoid ambiguity with
+ * V_ASN1_NEG. */
+#define V_ASN1_MAX_UNIVERSAL		0xff
 
 #define V_ASN1_UNDEF			-1
 #define V_ASN1_EOC			0
diff --git a/util/all_tests.json b/util/all_tests.json
index c621799..e4bd1ca 100644
--- a/util/all_tests.json
+++ b/util/all_tests.json
@@ -1,5 +1,6 @@
 [
 	["crypto/aes/aes_test"],
+	["crypto/asn1/asn1_test"],
 	["crypto/base64/base64_test"],
 	["crypto/bio/bio_test"],
 	["crypto/bn/bn_test"],