commit fa2cd1ee869e788cf0933252c4b4f6933e45f49e
author David Benjamin <davidben@google.com> Thu Feb 17 11:11:14 2022 -0500
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Mar 08 15:57:54 2022 +0000
tree 958822e2a4a0097f90f3f0efccfd1dabea27c38a
parent d258de72480b7538eaa5d1fa8faedaf2d4c8b5d1

Fix theoretical overflow in ASN1_INTEGER_cmp.

While unlikely, ASN1_STRING_cmp is allowed to return INT_MIN (by way of
memcmp), in which case negating would overflow.

Change-Id: Iec63a6acfad2c662493d22a0acea39ca630881c8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51630
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/asn1/a_int.c

diff --git a/crypto/asn1/a_int.c b/crypto/asn1/a_int.c
index 1695fd0..c94f574 100644
--- a/crypto/asn1/a_int.c
+++ b/crypto/asn1/a_int.c
@@ -72,22 +72,26 @@
 
 int ASN1_INTEGER_cmp(const ASN1_INTEGER *x, const ASN1_INTEGER *y)
 {
-    int neg, ret;
-    /* Compare signs */
-    neg = x->type & V_ASN1_NEG;
+    /* Compare signs. */
+    int neg = x->type & V_ASN1_NEG;
     if (neg != (y->type & V_ASN1_NEG)) {
-        if (neg)
-            return -1;
-        else
-            return 1;
+        return neg ? -1 : 1;
     }
 
-    ret = ASN1_STRING_cmp(x, y);
+    int ret = ASN1_STRING_cmp(x, y);
+    if (neg) {
+        /* This could be |-ret|, but |ASN1_STRING_cmp| is not forbidden from
+         * returning |INT_MIN|. */
+        if (ret < 0) {
+            return 1;
+        } else if (ret > 0) {
+            return -1;
+        } else {
+            return 0;
+        }
+    }
 
-    if (neg)
-        return -ret;
-    else
-        return ret;
+    return ret;
 }
 
 /*