From 2a768d04c697a10f1c0a8116c38ade9e8bf2a5d0 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Fri, 27 Oct 2017 10:37:27 -0700 Subject: [PATCH] Fix overflow checks when converting ASN.1 integers to long. (Credit to libFuzzer for finding this.) Change-Id: I0353d686d883703d39145c5bdd1e56368a587a35 Reviewed-on: https://boringssl-review.googlesource.com/22324 Reviewed-by: Adam Langley Reviewed-by: David Benjamin Commit-Queue: Adam Langley CQ-Verified: CQ bot account: commit-bot@chromium.org --- crypto/asn1/a_enum.c | 33 ++++++++++++++++++++++----------- crypto/asn1/a_int.c | 27 +++++++++++++++++++-------- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/crypto/asn1/a_enum.c b/crypto/asn1/a_enum.c index cc469055..4a779718 100644 --- a/crypto/asn1/a_enum.c +++ b/crypto/asn1/a_enum.c @@ -56,6 +56,7 @@ #include +#include #include #include @@ -110,7 +111,6 @@ int ASN1_ENUMERATED_set(ASN1_ENUMERATED *a, long v) long ASN1_ENUMERATED_get(ASN1_ENUMERATED *a) { int neg = 0, i; - long r = 0; if (a == NULL) return (0L); @@ -120,20 +120,31 @@ long ASN1_ENUMERATED_get(ASN1_ENUMERATED *a) else if (i != V_ASN1_ENUMERATED) return -1; - if (a->length > (int)sizeof(long)) { - /* hmm... a bit ugly */ - return (0xffffffffL); - } - if (a->data == NULL) - return 0; + OPENSSL_COMPILE_ASSERT(sizeof(uint64_t) >= sizeof(long), + long_larger_than_uint64_t); - for (i = 0; i < a->length; i++) { - r <<= 8; - r |= (unsigned char)a->data[i]; + if (a->length > (int)sizeof(uint64_t)) { + /* hmm... a bit ugly */ + return -1; } + + uint64_t r64 = 0; + if (a->data != NULL) { + for (i = 0; i < a->length; i++) { + r64 <<= 8; + r64 |= (unsigned char)a->data[i]; + } + + if (r64 > LONG_MAX) { + return -1; + } + } + + long r = (long) r64; if (neg) r = -r; - return (r); + + return r; } ASN1_ENUMERATED *BN_to_ASN1_ENUMERATED(BIGNUM *bn, ASN1_ENUMERATED *ai) diff --git a/crypto/asn1/a_int.c b/crypto/asn1/a_int.c index 617ba962..8a4edd69 100644 --- a/crypto/asn1/a_int.c +++ b/crypto/asn1/a_int.c @@ -57,6 +57,7 @@ #include #include +#include #include #include @@ -385,7 +386,6 @@ int ASN1_INTEGER_set(ASN1_INTEGER *a, long v) long ASN1_INTEGER_get(const ASN1_INTEGER *a) { int neg = 0, i; - long r = 0; if (a == NULL) return (0L); @@ -395,20 +395,31 @@ long ASN1_INTEGER_get(const ASN1_INTEGER *a) else if (i != V_ASN1_INTEGER) return -1; - if (a->length > (int)sizeof(long)) { + OPENSSL_COMPILE_ASSERT(sizeof(uint64_t) >= sizeof(long), + long_larger_than_uint64_t); + + if (a->length > (int)sizeof(uint64_t)) { /* hmm... a bit ugly, return all ones */ return -1; } - if (a->data == NULL) - return 0; - for (i = 0; i < a->length; i++) { - r <<= 8; - r |= (unsigned char)a->data[i]; + uint64_t r64 = 0; + if (a->data != NULL) { + for (i = 0; i < a->length; i++) { + r64 <<= 8; + r64 |= (unsigned char)a->data[i]; + } + + if (r64 > LONG_MAX) { + return -1; + } } + + long r = (long) r64; if (neg) r = -r; - return (r); + + return r; } ASN1_INTEGER *BN_to_ASN1_INTEGER(const BIGNUM *bn, ASN1_INTEGER *ai)